jackrabbit-oak-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jukka Zitting <jukka.zitt...@gmail.com>
Subject Re: svn commit: r1532157 - in /jackrabbit/oak/trunk: oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/nodetype/NodeTypeImpl.java oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/nodetype/NodeTypeEqualsTest.java
Date Tue, 15 Oct 2013 19:05:54 GMT

On Tue, Oct 15, 2013 at 2:47 PM, Tobias Bocanegra <tripod@apache.org> 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.

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.


Jukka Zitting

View raw message