Re: [jgrapht-developers] interface breaking changes
Brought to you by:
barak_naveh,
perfecthash
From: John V. S. <js...@gm...> - 2006-07-03 05:08:26
|
Notes on resolutions below, since I'm about to wrap up 0.7.0. Also, Eclipse and I went on a Ctrl-Alt-R purge last night and all the m_'s are gone forever! (Cue maniacal laughter...) Barak Naveh wrote: > Since 0.7.0 will anyway involves some interface-breaking changes, it might > be a good chance to consider fixing a few flaws were left lurking in Graph > -- probably due my fault... > > 1. Edge factory > ---------------------------- > The inclusion of the edge factory was a pre-Java 5.0 attempt to provide some > crude type-safety for edges. Now after generics have been applied, there > seem to be no strong need for the edge factory. The Graph interface can be > simplified by eliminating the edge factory. Consequently, addEdge(V,V) will > become obsolete and could be eliminate as well. Turns out edge factory is still required, because new E() doesn't work in Java generics. The default implementation is now ClassBasedEdgeFactory, which uses a class object to create new instances. > 2. NullPointerException -> IllegalArgumentException > --------------------------------------------------- > Some methods throw new NullPointerException. It might be better if they > throw new IllegalArgumentException to avoid confusion with the > NullPointerExceptions that the virtual machine may throw. See > http://pmd.sf.net/rules/strictexception.html for more on that. I disagree with PMD here. I think a NullPointerException is more natural than an IllegalArgumentException in this context, and java.util agrees (grep the src and you'll see them throwing many NPE's); since java.util has always been the model we follow, I think we should stick with the existing behavior. > 3. edgesOf(V v) > ------------- > Contract clarification: what if v is null? What if v doesn't exist in the > graph? Probably should through IllegalArgumentException. The code already throws NPE/IAE from getEdgeContainer, so I am just going to add that specification to the Javadoc; no behavior change. JVS |