On 03/20/09 12:38, Frank Schönheit wrote:
- KeySet.has, KeySet.remove, containsKey, containsValue, get, put, remove: You need to clarify how to compare the involved values for equality.

Why? Isn't this an implementation detail? In other words, shouldn't the
service implementing XMap specify this?
Hmm, okay, in this case your next request will be adding this to
css.container.Map :). Sadly, I don't have the slightest idea how I
should write in prose what (I think)
"css::uno::Any::operator==(css::uno::Any)" does ...

I would use structural induction over the format of UNO types (see <http://udk.openoffice.org/common/man/typesystem.html>).

- Values: "The map and the value collection are linked, in that changes in the map are reflected in the value collection, too." and "If you modify the map while an enumeration of values is running, the behavior of the enumeration is undefined." look like they are at odds with each other. What exactly is meant with "while an enumeration of values is running"?

Hmm ... That's an edge case, probably, depending on exactly the sentence
you pointed out.
Perhaps I should just state that operating on an XEnumeration object
which has been created (by XEnumerationAccess::createEnumeration)
*before* a change to the map happened yields undefined behavior. Would
that be better?

Looks better. However, you should then drop "The map and the value collection are linked, in that changes in the map are reflected in the value collection, too."

Also, the descriptions of the XEnumerationAccess base interface and KeySet should be changed to the clearer wording, too.

I also thought about simply requesting to invalidate (as in: will throw
a DisposedException when used) all enumerators (for keys, values, and
mapping pairs) once the map is modified. Would probably be the most
stringent requirement, but quite reasonable. What do you think?

Not sure whether this is easily implementable, so I would simply stick to undefined behavior.

- getSize: Is it needed? (At least theoretically, the return type poses a problem that could be prevented simply by leaving that method out.)

What's the problem with the return type?

What about a map that holds 2^31 elements?

- containsKey, containsValue, get, put, remove: Why the NullPointerException? Why would an implementation choose not to support null references to interfaces as keys and/or values? How would KeySet.has and KeySet.remove handle this?

Not supporting NULL for either keys or values could probably be
justified (for a concrete implementation) by the additional
implementation effort/complexity it would cost. Imagine an
implementation which internally uses the strict weak ordering on allowed
keys (or even allowed values, if it chooses a certain less-than-O(n)
implementation of containsValue) - allowing for NULL keys (values) would
make the implementation more difficult. So the implementor might decide
that that's just not worth it for the intended use cases.

I fail to see how null interface references complicate things here.

KeySet::has can only return false, since it does not allow for an
appropriate exception. Which, in some sense, is consistent with existing
XSet implementations - invalid keys cannot but treated as "not contained".

You meant "invalid keys cannot be treated as 'contained'"?

KeySet::remove does not have a problem, since that's unsupported, anyway
- so any other suitable exception can be thrown before actually checking
the passed key.

I thought it was KeySet.insert that is (implicitly) unsupported, not KeySet.remove?

Of course, even if this perhaps convinced you :), it might still be
debatable whether "Null*Pointer*Exception" really is the right thing
here. Again, this was inspired by java.util.Map.

I doubt that the Java inspiration is useful here. Null in Java is a rather different concept from null in UNO (e.g., only interface references can be null in UNO, while references to all non-primitive types can be null in Java).

- get: I would let it return Optional<any> and not raise NoSuchElementException, so that it can be used more naturally in multi-threaded scenarios.

Hmm. I don't like Optionals. In single-threaded scenarios, they
unnecessarily complicate the usage.

Hm, I don't like exceptions, they unnceessarily complicate the usage... ;)

- put: Better let it return Optional<any>. What exactly is meant with "@return [...] <NULL/> if there was no such previous association"?

In the complete wording, it is

  @return
    the value which was previously associated with the given key, or
    <NULL/>, if there was no such previous association.

so, I had hope this is read as "no mapping existed for the given Key".

Sorry, what I meant is that "<NULL/>" is not a valid value for a UNO any.

- remove: An alternative would be to let it return Optional<any> and not raise NoSuchElementException (again, this might be more appropriate in certain multi-threaded scenarios).

Okay, we have "I would let it return Optional...", "Better let it return
Optional..." and "An alternative would be to let it return Optional...".
Can you decide for one of those? :)

No, the three cases are rather different.

- create, createImmutable: You need to clarify what "unsupported types" are.

Done.

- Null keys not being allowed should be part of the interface type list item, not a list item of its own.

- There is no UNO type named "int".

- For my tastes, these rules are too arbitrary for this "default implementation."

- create: You need to clarify whether and when put on the resulting XMap will raise IllegalArgumentException.

Done. Please review, I also added a new paragraph about accepting value
types.

- There is no "the <NULL/> value" in UNO. (Rather, for every interface type T, there is a null value of that type.)

- "If the value type's class if" typo.

- There are no UNO types with TypeClass::UNION or TypeClass::UNKNOWN. (com.sun.star.uno.TypeClass is in a poor state. I would not mention it at all in the definition of XMap/Map.)

-Stephan

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to