Message2939

Author byronf
Recipients
Date 2007-12-05.06:27:51
SpamBayes Score
Marked as misclassified
Message-id
In-reply-to
Content
> The internal view collections of PyMapEntrySet and
> PyMapKeyValSet are both expecting to extend PyMapSet, but there
> is no such class in the patch.  I'm guessing PyMapCollection
> was supposed to be renamed to PyMapSet?

Sorry, I goofed.  There was a stale class on my side that allowed
it to still compile.  Fixed.

> From a cursory examining, it seems like PyMapCollection/Set
> could be greatly simplified by extending AbstractSet rather
> than implementing the whole interface on its own.  Many of the
> implemented methods could just go away.

Not Really, the problem is that (Which is true in general)
objects retrieved from the dictionary in java code must pass
through __tojava__, and java object being passed to the
dictionary must use Py.java2py which utilizes the plugable
PyObjectAdapter system. Using AbstractSet would eliminate size()
and isEmpty(), but I think that's about it.  Allot of the code
implements the obscure java.util.Map#entrySet method which will
probably never get used, but I added it for completeness.

> The iterator implementation is identical between the two Set
> classes, so it should be shared in the super class.

Done

> Unless there's a name conflict, a Java class should be imported
> with an import statement and just its short name should be used
> in the code.  I see the full use of java.util.Iterator.

Done

> I'd move the values method down into the Map implementation
> section now that it belongs there.

Done

> It'd be better to move Dict2JavaTest into Lib/src/javatests so
> it's separated out with the tests.

I found a Lib/test/javatests which I moved Dict2JavaTest to.
Done

> Tests using unittest.TestCases are much preferred to a list of
> asserts as in test_dict2java.py.  With a list of asserts, the
> first test failure means the rest of the tests aren't run which
> makes it harder to track down errors.

Done
History
Date User Action Args
2008-02-20 17:18:52adminlinkissue1817565 messages
2008-02-20 17:18:52admincreate