[ 
https://issues.apache.org/jira/browse/JCR-680?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12462051
 ] 

Jukka Zitting commented on JCR-680:
-----------------------------------

> it might be just my lack of understanding but the proposed redesign
> is IMO more complex and harder to grasp than gthe current implementation. 

I would argue for my proposal (GenericValue.patch) with the following:

a) The proposed Value implementation (counting the essential *Value classes + 
ValueFactoryImpl) totals 379 NCSS (non-commenting source statements), a notable 
reduction (-39%) from the current 619 NCSS. Also the number of methods per 
class is down as well as virtually all method-level code complexity metrics.

b) All the member variables of instantiated objects are always used in the 
proposed implementation, whereas almost all the member variables of the current 
implementation are unused in some object states.

c) Related to b), the proposed implementation contains only a single mutable 
member variable (GenericValue.value) and only the GenericValue instances are 
mutable. The current implementation has four mutable member variables and all 
the *Value instances are mutable.

d) The proposed implementation follows a well known and documented design 
pattern.

e) The proposed implementation fixes JCR-320 trivially.

> i am not against redesigning the Value implementations in general but
> i frankly fail to see the net benefit of the proposed redesign. it's harder
> to understand (at least for me ;)

I'm not too committed to the proposal, so it's OK to me not to apply it if the 
above rationale is not convincing enough to justify the drawbacks of changing 
existing code without any immediate need other than JCR-320 (which is quite 
minor).

> btw: the patch is still quite big and quite difficult to read.

Check the attached GenericValue.tar.gz file, it contains the proposed versions 
of the modified classes. I find the resulting source files much easier to read 
in this case since a large part of the patch is simply removing code that's no 
longer needed.


> Improve the Value implementation
> --------------------------------
>
>                 Key: JCR-680
>                 URL: https://issues.apache.org/jira/browse/JCR-680
>             Project: Jackrabbit
>          Issue Type: Improvement
>          Components: core
>            Reporter: Jukka Zitting
>         Assigned To: Jukka Zitting
>            Priority: Minor
>         Attachments: class.jpg, GenericValue.patch, GenericValue.tar.gz, 
> JCR-680.patch
>
>
> The current Value implementation found in jackrabbit-jcr-commons has some 
> deficiencies like Value.equals() being incorrect in some cases (see for 
> example JCR-320), and Name and Path values not following namespace remappings.

-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: 
https://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

Reply via email to