commons-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "ASF GitHub Bot (JIRA)" <>
Subject [jira] [Commented] (TEXT-80) StrLookup API confusing
Date Thu, 08 Jun 2017 12:02:19 GMT


ASF GitHub Bot commented on TEXT-80:

Github user britter commented on the issue:
    @chtompki this indicates a source incompatible release. Binary compatible means that you
can swap out the 1.1 jar with the 1.2 jar without a recompile. Would that work?
    I can't recall our policy for source incompatible changes. But I think for example adding
new methods to interfaces has been okay in the past. This is also source incompatible (recompilation
fails) but binary compatible (swapping out one jar with another works).

> StrLookup API confusing
> -----------------------
>                 Key: TEXT-80
>                 URL:
>             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

View raw message