[ 
https://issues.apache.org/jira/browse/LANG-863?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13811539#comment-13811539
 ] 

Sebb commented on LANG-863:
---------------------------

Layout looks generally good, no tabs.
The code looks neat, and implements the requirements (as far as they were 
provided).

Some minor nits:

One file (the main code) is missing tne AL header.

I think the tests need to include Object.class, Object.class.
And ideally the testDistance() method should be split into more tests, for 
example null, Object, equal Class etc.
Lang uses generics, so Class should really be Class<?>.

==

As a separate issue, I'm not sure the requirements are fully specified.
Does it make sense for null to match anything? Maybe that should return -1 
rather than 0.
Likewise for unrelated classes, why should String and Boolean have a valid 
inheritance number?
At the moment distance(String, Boolean) == distance(Boolean, String) == 0
That does not seem very useful to me.
Code that use the distance is probably going to have to check to see if 0 means 
equal classes or disjoint classes or null.
Whereas if the code returns -1 for invalid inheritance, it should make the 
caller's job easier.

> Method returns number of inheritance hops between parent and subclass
> ---------------------------------------------------------------------
>
>                 Key: LANG-863
>                 URL: https://issues.apache.org/jira/browse/LANG-863
>             Project: Commons Lang
>          Issue Type: New Feature
>          Components: lang.reflect.*
>            Reporter: Daneel S. Yaitskov
>             Fix For: Patch Needed
>
>         Attachments: LANG-863.patch
>
>   Original Estimate: 1h
>  Remaining Estimate: 1h
>
> For example.
> class A {
> }
> class B extends A {
> }
> class C extends B {
> }
> int d;
> d = InheritanceUtils.distance(A.class, A.class);
> Assert.assertEquals(0, d);
> d = InheritanceUtils.distance(B.class, A.class);
> Assert.assertEquals(1, d);
> d = InheritanceUtils.distance(C.class, A.class);
> Assert.assertEquals(2, d);



--
This message was sent by Atlassian JIRA
(v6.1#6144)

Reply via email to