> A few notes in general:
>
> 1. In general, the changes are good and beneficial. I think
> that is important to say in advance.
And they were easy to do without expected side effects.
> 2. These changes should have been communicated to the project
> manager of the Optimization project (me). In particular since
> you there are changes to the core, and in running projects
> (i.e. packaging). Also a new class was added to
> org.mmbase.util. All this is no problem in and of itself, but
> the project maintainer should have been informed, so it can
> be documented (as step 11 in the Optimixation project) and
> the list would be informed that changes wre being made. This
> prevents comments later on.
I expected this note, because I have had this one already a couple of times.
First, I didn't apply code conventions and javadoc as step 11 describes. I
removed a lot of unused lines of code and improved some methods.
Second, this is exactly why I don't actively develop on the mmbase codebase.
When I am at work I implement applications on top of mmbase. It does what I
need so I don't have to modify the code. Sometimes, I have my weak moments
and develop things in my own time. I want to do it when I have the time and
not when the mmbase community wants my time. IOW I don't want to wait for
approval for something I believe does not have any major impact. I am not
going to wait a couple of days before I am allowed to do it. The moment has
passed and I probably will spend my time in some other way.
Third, I did this on the head. It is THE place to have many changes to the
code. Users (other developers) of mmbase trust us to change to code in a
good way. They know when they switch to a new major version (I think the 1.x
are) that they have to do some work to get it working again, but that it
will be a better system as the previous version.
Fourth, IMO, changing user functionality should be commnicated, but
refactoring code to have a more robust and stable system could happen at
anytime. There are a lot of good experienced developers watching mmbase who
don't want to go through all the bureaucratic procedures to change some
methods they know could be better.
> 3. A few of the comments you made are valid and should
> possibly be included in the MMBase code conventions. I'll
> look into these.
Thank you,
While I am writing this we might need another document for people who want
to contribute to mmbase and want a quick start. A hacker's guide which tells
how to participate and where documentation is and what rules apply. Such a
document could contain something like the following.
* Participating in the community - short description hwo to communicate
help
* What to read
* Directory layout
* Coding style - explaining most important ones and redirect to full list
* Document everything - which parts of the system should be documented
* Error message conventions
* Other conventions
* Exception handling
* Automated tests
* Writing test cases before code
* Writing log messages
* Patch submission guidelines
* Filing bugs / issues
* Commit access
* Release numbering, compatibility, and deprecation
* Stabilizing and maintaining releases
> 4. A few changes may have been a bit hasty - I can't tell
> until I further investigated, will mail later. These are all
> minor issues though.
I don't think so. Some of them don't comply fully with the mmbase coding
standards, but I didn't do it blindly.
> Specific comments:
>
> > 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.
>
> Agreed. Before we cahneg anything though, it is important to
> determine where it is used now and what changign teh
> implementation would mean for backward compatibility.
Backward compatibility? Fixing the equals will make the system more robust
and everyone wants that.
> > There are catch blocks in the code without an
> implementation. Some are
> > commented, but most of them are not. A good practice is to add
> > log.debug("",
> > exception) when nothing else should happen.
>
> Or, in some cases, log.trace().
That's even better ;)
> > Mmbase really has to improve in exception handling in the
> internals.
> > Most of the time Throwable, Error, RuntimeException or
> Exception are
> > caught instead of the more specific types. If they are not
> caught the
> > declared throws class mentions the generic exception class.
> This would
> > represent a loss of possibly important information. The
> throws claus
> > should be as specific as possible without grouping of exceptions.
>
> Agreed, we should look into this closely. Noet tahts oem code
> simply doesn't 'want' to throw exceptions at all. We should
> re-evaluate if that is desireable, posisbyl ona a case-by-case basis.
>
> > Specify the minimum visibility scope that a method requires.
>
> This is MMBase code convention. Some code has already been
> marked (with
> @scope) for changing.
> Note that not everyone fully agrees with this pracice, and in
> some cases (i.e. objects that are intended to just hold data
> and only have fields) it may actually be useful.
You mean value-objects? What is the use of a value-object if the accessors
are not public? The rule applies for objects implementing behaviour.
> > 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.
>
> Also included in the MMBase code conventions. The main issue
> here is backward compatibility with older code that
> uses/expects Vector and Hashtable.
backward compatibility again. Yes, we have to provide a durable api, but
aren't we allowed to change the api between major releases?
> > Some conclusion can be drawn from these import listing.
> > - many imports of the same package usually means that the
> class is in
> > the wrong package.
>
> Though not always, i.e. java.util or the interface packages,
> and the mmbase core package are much used and I don't think
> it adds much to specify all classes.
java.util contains utilities (read behaviour) and you don't need a lot of
different behaviour in an object. If you do then you have to much
functionality. With interface packages you are referring to the bridge? That
package exports to many types. I think I would have split that one up into
multiple packages. The same applies for the core classes.
Nico
_______________________________________________
Developers mailing list
[email protected]
http://lists.mmbase.org/mailman/listinfo/developers
