Nico Klasens wrote: > This weekend I reviewed and cleaned the code a little of MMBase. After > reviewing I decided to only do some of the stuff now and leave the rest for > another afternoon. When I reviewed the code I came to the conclusion that > the quality wasn't as bad as I have seen in some other codebases. Some best
How nice :-) > One equals method I didn't like at all, was the one in MMObjectNode, but I > haven't changed that one. We really should discuss what this should > be. That a very odd one indeed, and I may be the culprit here. I would like to base equals always on the number. That is not the case now (ussually the equals of Object is used). But you can override it (which must as usual for every method in MMObjectNod be done in MMObjectBuilder), because sometimes it is absolutely essential that equals is based on the number. I would be +1 for generalizing this behaviour, because it would e.g. ensure that the same node does not appear more then once in the node-cache. > > Usage of modifiers (Public, protected, private, final and synchronize) > ============================================================================ > =========== > Using public, protected, private or package scope > Specify the minimum visibility scope that a method requires. THE trick in OO > is to hide as much of the implementation secrets as possible. The visibility > modifiers are there to accomplish this. Remember that there are four scopes, > not two (public and private) Of course. But I am still not completely detemeined on the use of private, protected en package on certain methods. Sometimes we see that methods are made package only to make sure that the test-cases have access, which always stroke me as a kind of odd reason. Furthermore there often can be some discussion whether a method must be private or protected. When private, you can easier safely change the implementation of the class, but when protected, you often offer greater flexibility for extension. It can be very irritating if methods were made private which you would very much like to call or override in your exension. I figure that common sense should determin what to choose. If you envision a complete change of the class in which the method may get lost, because it is very specific or perhaps oddly implmented, I'd make it private. If the methods looks good, and it is imaginable that someone wants to change or use its behavior for some reason, I normally go for protected. But I wonder if we good agree on some rules of thumb for this? > > Declaration of overspecific variables > ============================================================================ > =========== > Like I wrote above, OO is all about encapsulation and hiding > implementations. This also applies for methods and variables. A change in OO > code should have a minimal impact on the code. The idiom of choice here is > to choose the highest level of abstraction appropriate to the problem. This > means that only the interfaces (like List and Map) for collections should be > used for method return types, method parameters, fields and local variables. And if I may add, the logically most apt one. E.g. I think that NodeManager.getFields() should have returned a Set or SortedSet, and not a List, because it is logically impossible that a builder has two the same fields. Michiel -- Michiel Meeuwissen mihxil' Mediacentrum 140 H'sum [] () +31 (0)35 6772979 nl_NL eo_XX en_US _______________________________________________ Developers mailing list Developers@lists.mmbase.org http://lists.mmbase.org/mailman/listinfo/developers