On 16.06.2010 10:57, Bjoern Michaelsen wrote:
Am Wed, 16 Jun 2010 10:11:25 +0200
schrieb Mathias Bauer<nospamfor...@gmx.de>:
BTW: I don't remember if any of the two ItemSet implementations
returns an VoidItem in a GetItem call for a disabled item or if NULL
is returned then. IMHO it always should be the latter.
Getting a void item the old implementation returns:
- a state disabled and leaves the item pointer unmodified in
GetItemState
- a NULL item pointer in GetItem
- a pointer to the void item and fires an assertion (triggered on which
id == 0 _OR_ type == SfxVoidItem) in Get
Getting a void item the new implementation returns:
- state disabled and leaving the pointer unmodified only on which id = 0
in GetItemState
- a NULL item pointer on which id = 0 in GetItem
- a pointer to the item in Get
The last action looks wrong to me. In the same way as an ItemSet can't
return anything if the item is invalid, it shouldn't return something if
it is disabled. Unfortunately changing that might be risky. But sometimes...
Void items with a which id set to the which position it is in in the
itemset are treated just as any other item. Assertions are mostly pushed
to the point where such items are put in the set, not when they are
extracted -- when it is too late already.
This is correct IMHO. The problem is the "hidden message" behind an
SfxVoidItem if its ID is 0.
Back to your primary topic. If we identify the places where a
VoidItem with ID != 0 is put, and if the developers state that this
is by intent, we can remove that assertion and make putting VoidItems
with ID != 0 a valid action that does not disable that item.
True. Still somewhere code may lurk, where a void item is put with a
which id set and expected to work as a disabled state. For example IIRC
doing a SfxItemSet::Put(SfxVoidItem(4711)) will result in
SfxItemSet::GetItemState(4711, false, NULL) returning a disabled state
in the old implementation, but not in the new one.
So there are three cases. The developer intended:
- the behavior of the old implementation (which would be abuse of the
interface)
- the behavior of the new implementation (which would be a bug)
- any behavior, because they make no difference in the given scenario.
A migration path could be as follows: We have a aborting assertion on
putting void items. Every such call either gets replaced by:
- an explicit DisableItem call
- or an explicit (new) PutVoidItem call (with the which id of the void
item set to the which id of the position in the set)
Assertions don't help, you can never be sure that you have fixed
everything. But OTOH I don't see any way how we can let the compiler do
the work for us.
Regards,
Mathias
--
Mathias Bauer (mba) - Project Lead OpenOffice.org Writer
OpenOffice.org Engineering at Sun: http://blogs.sun.com/GullFOSS
Please don't reply to "nospamfor...@gmx.de".
I use it for the OOo lists and only rarely read other mails sent to it.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@openoffice.org
For additional commands, e-mail: dev-h...@openoffice.org
