Message2936
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?
Those two classes also need to implement Set to be acceptable as return values from keySet and entrySet, so perhaps PyMapSet was supposed to implement Set? 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.
The iterator implementation is identical between the two Set classes, so it should be shared in the super class.
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.
I'd move the values method down into the Map implementation section now that it belongs there.
It'd be better to move Dict2JavaTest into Lib/src/javatests so it's separated out with the tests.
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.
I haven't run the tests yet since this doesn't compile, but if you'll fix the first few errors I can run the tests and really evaluate the patch. The suboptimal use of asserts isn't a deal breaker, it'd just be nice if it were fixed. |
|
Date |
User |
Action |
Args |
2008-02-20 17:18:52 | admin | link | issue1817565 messages |
2008-02-20 17:18:52 | admin | create | |
|