Hi, On Tue, Oct 15, 2013 at 12:05 PM, Jukka Zitting <[email protected]> wrote: > Hi, > > On Tue, Oct 15, 2013 at 2:47 PM, Tobias Bocanegra <[email protected]> wrote: >> Sure, but in this particular case I think that comparing the actual >> definition is desired. Imagine a NodeType editor that operates on a session >> and wants to list the nodetypes that were modified. > > Makes sense. > >> OTOH, in most cases comparing the names should be good enough. If you think >> the comparing the CND is overhead, please re-open [0] and I'll change it. > > CND comparison should be good here. > > In fact the main change I'd make to the implementation is to drop the > caching of the CND string, and simply regenerate it whenever needed. > That should simplify the equals method to: > > public boolean equals(Object o) { > return o instanceof NodeTypeDefinition > && getCnd(this).equals(getCnd((NodeTypeDefinition) o)); > } > > I'd value simplicity over performance here as AFAICT no > performance-sensitive code uses this feature and correctly > invalidating the cached value would be non-trivial. good point.
> Also, as a lesser concern, I'd drop the fallback in the "catch > (IOException e)" clause. Instead of logging the error and continuing > with different semantics, I think it would be better to just fail fast > with an IllegalStateException. ok. regards, toby
