commons-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "ASF GitHub Bot (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (TEXT-80) StrLookup API confusing
Date Mon, 05 Jun 2017 17:46:04 GMT

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

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

Github user ameyjadiye commented on the issue:

    https://github.com/apache/commons-text/pull/44
  
    Hi @garydgregory , Its already discussed here [LANG-564](https://issues.apache.org/jira/browse/LANG-564)
and [TEXT-80](https://issues.apache.org/jira/browse/TEXT-80) and all seems agree on changing
the code, according to discussion code proposed in this PR will improve the API. 
    
    As this is moved from commons lang, version of  ```StrLookup``` will soon be deprecated.
so no cross module impact I see for now and yes whoever using text 1.0, 1.1 will need to change
code very minimal. 


> 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)

Mime
View raw message