|
⇤ ← Revision 1 as of 2005-02-26 19:16:29
Size: 2234
Comment:
|
Size: 1669
Comment:
|
| Deletions are marked like this. | Additions are marked like this. |
| Line 9: | Line 9: |
| `PySequence` excerpt: {{{ /** * The abstract superclass of PyObjects that implements a Sequence. * Minimize the work in creating such objects. * * Method names are designed to make it possible for PySequence to * implement java.util.List interface when JDK 1.2 is ubiquitous. * * All subclasses must declare that they implement the ClassDictInit * interface, and provide an classDictInit() method that calls * PySequence.classDictInit(). * * Subclasses must also implement get, getslice, and repeat methods. * * Subclasses that are mutable should also implement: set, setslice, del, * and delRange. */ |
I had originally put in here a detailed anaysis of conflicts between the comments and the source code regarding the `InitModule` interface. The comments seemed to indicate that `PySequence` should implement `InitModule` but couldn't because the class was abstract and `PyJavaClass` would try to instatiate it erroneously. But `PyJavaClass` was already correctly using reflection to give an error message in `PyObject __call__(PyObject[] args, String[] keywords)` if an abstract class was passed in. So then I was trying to track down why the `PyList` wasn't implementing `ClassDictInit`. It was providing the required static method classDictInit() but it was a do-nothing method and it wasn't calling `PySequence.classDictInit()` like the comments said it should. So then I diff'd the changes from 2.2a to the tip with new style class changes. Apparently, all this changed with new style classes. A case of comments being 2 to 3 generations old. |
| Line 28: | Line 11: |
| // this class doesn't "implement InitModule" because otherwise // PyJavaClass.init() would try to instantiate it. That fails because this // class is abstract. TBD: is there a way to test for whether a class is // abstract? abstract public class PySequence extends PyObject { /** * This constructor is used by PyJavaClass.init() */ public PySequence() {} }}} Sure there's a way to do it using reflection, and looking at `public PyObject __call__(PyObject[] args, String[] keywords)` shows that there is already implemented and that an error is reported if called on an abstract class. Does this mean `PySequence` should now implement `InitModule`, or actually `ClassDictInit` since `InitModule` is deprecated? |
<whine>[[BR]]My personal opinion: *Under-commenting code is bad (only comment the unobvious) *Over-commented code can also be bad, and potentially leads to... *Wrong/Out-Of-Date comments are worst of all |
| Line 40: | Line 16: |
| Then, consider the comment that says "All subclasses must declare they implement `ClassDictInit` interface and provide an classDistInit() method that calls `PySequence.classDictInit()`". If you look at `PyList`, it '''doesn't''' implement `ClassDictInit`, but it does have a `public static void classDictInit(PyObject dict)` method. However, it is an empty method that '''doesn't''' call `PySequence.classDictInit()`. | This wasn't a case of over-commenting, just a case of not maintaining the comments with the code. |
| Line 42: | Line 18: |
| Either: *the design has evolved and the comments are out of date *`PySequence` and/or `PyList` (and probably other `PySequence` subclasses) aren't doing something they should be doing *both of the above |
</whine> Of course, if I knew more about the code to begin with, I probably would have figured it out faster. But the comments are supposed to be there help the neophytes, no? |
Notes/Discussion/Decisions on having PySequence implement java.util.List...
Topics
[#initmodule InitModule]
Topic: InitModule BR Anchor(initmodule) ClarkUpdike Feb 26 2005 BR I had originally put in here a detailed anaysis of conflicts between the comments and the source code regarding the InitModule interface. The comments seemed to indicate that PySequence should implement InitModule but couldn't because the class was abstract and PyJavaClass would try to instatiate it erroneously. But PyJavaClass was already correctly using reflection to give an error message in PyObject __call__(PyObject[] args, String[] keywords) if an abstract class was passed in. So then I was trying to track down why the PyList wasn't implementing ClassDictInit. It was providing the required static method classDictInit() but it was a do-nothing method and it wasn't calling PySequence.classDictInit() like the comments said it should. So then I diff'd the changes from 2.2a to the tip with new style class changes. Apparently, all this changed with new style classes. A case of comments being 2 to 3 generations old.
<whine>BRMy personal opinion:
- Under-commenting code is bad (only comment the unobvious)
- Over-commented code can also be bad, and potentially leads to...
- Wrong/Out-Of-Date comments are worst of all
This wasn't a case of over-commenting, just a case of not maintaining the comments with the code.
</whine>
Of course, if I knew more about the code to begin with, I probably would have figured it out faster. But the comments are supposed to be there help the neophytes, no?