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

ASF GitHub Bot commented on TEXT-80:
------------------------------------

Github user chtompki commented on the issue:

    https://github.com/apache/commons-text/pull/44
  
    @britter  - I would have to test it out, but fair point.
    
    It feels like we should always maintain source and binary backwards 
compatibility for minor version updates. But, if the policy is different from 
that feeling, then I'm not opposed to the changes.
    
    @garydgregory - What is the policy on source incompatible changes in minor 
version updates?


> StrLookup API confusing
> -----------------------
>
>                 Key: TEXT-80
>                 URL: https://issues.apache.org/jira/browse/TEXT-80
>             Project: Commons Text
>          Issue Type: Bug
>            Reporter: Etienne Neveu
>             Fix For: 1.x
>
>
> [bayard: copying this from LANG-564]
> I don't see the point of having a generic type parameter on the StrLookup 
> class, if it's not used anywhere. No method / field in StrLookup references 
> this type parameter. IntelliJ IDEA itself reports a warning: "Type parameter 
> 'V' is never used". Moreover, Java generics are not reified, so there is no 
> reliable way to access the type parameter at runtime (and I don't see the 
> point of doing that anyway...).
> While the Javadoc tries to clarify the purpose of a StrLookup, the unused 
> type parameter is still confusing, and the client code has to un-necessarily 
> specify type parameters. For example, I have to write:
> StrLookup<?> lookup = StrLookup.noneLookup();
> StrLookup<String> lookup2 = StrLookup.systemPropertiesLookup();
> StrLookup<Integer> lookup3 = StrLookup.mapLookup(integerMap);
> instead of
> StrLookup lookup = StrLookup.noneLookup();
> StrLookup lookup2 = StrLookup.systemPropertiesLookup();
> StrLookup lookup3 = StrLookup.mapLookup(integerMap);
> My best guess is that this type parameter was added when commons-lang was 
> generified, because StringLookup.mapLookup() takes a generified Map. Doing 
> this is not really needed, though: we could remove the <V> type parameter 
> everywhere, and replace the StrLookup.mapLookup()'s Map<String, V> with a 
> Map<String, ?> (which is the same as Map<String, ? extends Object>, but 
> shorter).
> I guess it's too late to change this now, due to backward compatibility... 
> But I thought I'd comment just in case it's still possible.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

Reply via email to