[jira] [Commented] (TEXT-80) StrLookup API confusing

[ASF GitHub Bot (JIRA)]

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

ASF GitHub Bot commented on TEXT-80:


Github user asfgit closed the pull request at:

https://github.com/apache/commons-text/pull/44


> StrLookup API confusing
> ---
>
> Key: TEXT-80
> URL: https://issues.apache.org/jira/browse/TEXT-80
> Project: Commons Text
>  Issue Type: Bug
>Reporter: Etienne Neveu
>Assignee: Pascal Schumacher
>Priority: Major
> Fix For: 1.3
>
>
> [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 lookup2 = StrLookup.systemPropertiesLookup();
> StrLookup 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  type parameter 
> everywhere, and replace the StrLookup.mapLookup()'s Map with a 
> Map (which is the same as Map, 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
(v7.6.3#76005)

[jira] [Commented] (TEXT-80) StrLookup API confusing

[ASF GitHub Bot (JIRA)]

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

ASF GitHub Bot commented on TEXT-80:


Github user PascalSchumacher commented on the issue:

https://github.com/apache/commons-text/pull/44
  
I'm closing this as `StrLookup` will deprecated and replaced with 
`StringLookup` in the next release.


> StrLookup API confusing
> ---
>
> Key: TEXT-80
> URL: https://issues.apache.org/jira/browse/TEXT-80
> Project: Commons Text
>  Issue Type: Bug
>Reporter: Etienne Neveu
>Assignee: Pascal Schumacher
>Priority: Major
> Fix For: 1.3
>
>
> [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 lookup2 = StrLookup.systemPropertiesLookup();
> StrLookup 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  type parameter 
> everywhere, and replace the StrLookup.mapLookup()'s Map with a 
> Map (which is the same as Map, 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
(v7.6.3#76005)

[jira] [Commented] (TEXT-80) StrLookup API confusing

[ASF GitHub Bot (JIRA)]

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

ASF GitHub Bot commented on TEXT-80:


Github user ameyjadiye commented on the issue:

https://github.com/apache/commons-text/pull/44
  
Lets park this for 2.X release.


> 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 lookup2 = StrLookup.systemPropertiesLookup();
> StrLookup 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  type parameter 
> everywhere, and replace the StrLookup.mapLookup()'s Map with a 
> Map (which is the same as Map, 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.4.14#64029)

[jira] [Commented] (TEXT-80) StrLookup API confusing

[Gary Gregory (JIRA)]

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

Gary Gregory commented on TEXT-80:
--

Hi All,

We all know that maintaining binary compatibility in a minor release is 
paramount.

Breaking sources in a minor release is an incredible annoyance to users. In 
this case, IMO, the change is not important enough to warrant the break. We 
will also avoid emails and bug reports that we broke sources for no compelling 
reason.

Gary

> 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 lookup2 = StrLookup.systemPropertiesLookup();
> StrLookup 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  type parameter 
> everywhere, and replace the StrLookup.mapLookup()'s Map with a 
> Map (which is the same as Map, 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)

[jira] [Commented] (TEXT-80) StrLookup API confusing

[ASF GitHub Bot (JIRA)]

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

ASF GitHub Bot commented on TEXT-80:


Github user coveralls commented on the issue:

https://github.com/apache/commons-text/pull/44
  

[![Coverage 
Status](https://coveralls.io/builds/11887530/badge)](https://coveralls.io/builds/11887530)

Coverage remained the same at 96.653% when pulling 
**fff3f43d27ed831986b36691eaeb7570eaa94d63 on ameyjadiye:TEXT-80** into 
**5f498c0f4783d035bfeb77517731c948f8567b1e on apache:master**.



> 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 lookup2 = StrLookup.systemPropertiesLookup();
> StrLookup 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  type parameter 
> everywhere, and replace the StrLookup.mapLookup()'s Map with a 
> Map (which is the same as Map, 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)

[jira] [Commented] (TEXT-80) StrLookup API confusing

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/TEXT-80?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=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 lookup2 = StrLookup.systemPropertiesLookup();
> StrLookup 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  type parameter 
> everywhere, and replace the StrLookup.mapLookup()'s Map with a 
> Map (which is the same as Map, 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)

[jira] [Commented] (TEXT-80) StrLookup API confusing

[ASF GitHub Bot (JIRA)]

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

ASF GitHub Bot commented on TEXT-80:


Github user britter commented on the issue:

https://github.com/apache/commons-text/pull/44
  
@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: 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 lookup2 = StrLookup.systemPropertiesLookup();
> StrLookup 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  type parameter 
> everywhere, and replace the StrLookup.mapLookup()'s Map with a 
> Map (which is the same as Map, 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)

[jira] [Commented] (TEXT-80) StrLookup API confusing

[ASF GitHub Bot (JIRA)]

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

ASF GitHub Bot commented on TEXT-80:


Github user chtompki commented on the issue:

https://github.com/apache/commons-text/pull/44
  
@britter if I declare a class
```java
package com.rt;

import org.apache.commons.text.StrLookup;

public class TextTester extends StrLookup {

public String lookup(String key) {
return null;
}

}
```
on commons-text:1.1, and up-version into these changes, I get compiler 
errors. To me, that implies that we must do a major version release to 
accommodate them.


> 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 lookup2 = StrLookup.systemPropertiesLookup();
> StrLookup 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  type parameter 
> everywhere, and replace the StrLookup.mapLookup()'s Map with a 
> Map (which is the same as Map, 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)

[jira] [Commented] (TEXT-80) StrLookup API confusing

[ASF GitHub Bot (JIRA)]

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

ASF GitHub Bot commented on TEXT-80:


Github user britter commented on the issue:

https://github.com/apache/commons-text/pull/44
  
@ameyjadiye the best would be to rebase your branch against master an dann 
do a force push. You can do it like this (if you have configured this 
repository as remote with name upstream):

```shell
git checkout master
git fetch upstream
git merge upstream/master
git checkout TEXT-80
git rebase master
git push -f
```

This will bring your branch up to date with master and trigger a new CI 
build.


> 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 lookup2 = StrLookup.systemPropertiesLookup();
> StrLookup 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  type parameter 
> everywhere, and replace the StrLookup.mapLookup()'s Map with a 
> Map (which is the same as Map, 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)

[jira] [Commented] (TEXT-80) StrLookup API confusing

[ASF GitHub Bot (JIRA)]

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

ASF GitHub Bot commented on TEXT-80:


Github user ameyjadiye commented on the issue:

https://github.com/apache/commons-text/pull/44
  
Hi @britter, there is no issue even in checkstyle with my changes,  above 
Travis build failed because there was some trailing space issue in master and 
which you have already fixed. merging this will pass everything clean *OR* you 
want me to do some dummy commit so Travis will trigger again and clear  error 
on this PR ? I don't think that's needed though.


> 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 lookup2 = StrLookup.systemPropertiesLookup();
> StrLookup 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  type parameter 
> everywhere, and replace the StrLookup.mapLookup()'s Map with a 
> Map (which is the same as Map, 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)

[jira] [Commented] (TEXT-80) StrLookup API confusing

[ASF GitHub Bot (JIRA)]

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

ASF GitHub Bot commented on TEXT-80:


Github user britter commented on the issue:

https://github.com/apache/commons-text/pull/44
  
@chtompki did you run chirr manually? Because it was checkstyle which 
caused the Travis build to fail (trailing white spaces). I think this change 
should not break BC. @ameyjadiye can you please fix the checkstyle errors and 
push again?


> 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 lookup2 = StrLookup.systemPropertiesLookup();
> StrLookup 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  type parameter 
> everywhere, and replace the StrLookup.mapLookup()'s Map with a 
> Map (which is the same as Map, 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)

[jira] [Commented] (TEXT-80) StrLookup API confusing

[ASF GitHub Bot (JIRA)]

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

ASF GitHub Bot commented on TEXT-80:


Github user ameyjadiye commented on the issue:

https://github.com/apache/commons-text/pull/44
  
Hi @chtompki , since commons-text is relatively new there are very [low 
usage](https://mvnrepository.com/artifact/org.apache.commons/commons-text), 
```clirr:check``` is also passed whoever is using it and wants to bump version 
needs minor change in their code, but its ok we can park it till 2.X release.


> 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 lookup2 = StrLookup.systemPropertiesLookup();
> StrLookup 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  type parameter 
> everywhere, and replace the StrLookup.mapLookup()'s Map with a 
> Map (which is the same as Map, 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)

[jira] [Commented] (TEXT-80) StrLookup API confusing

[ASF GitHub Bot (JIRA)]

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

ASF GitHub Bot commented on TEXT-80:


Github user chtompki commented on the issue:

https://github.com/apache/commons-text/pull/44
  
The change is generally all right with me, but we can't release this until 
a 2.X release though because of signature changes.


> 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 lookup2 = StrLookup.systemPropertiesLookup();
> StrLookup 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  type parameter 
> everywhere, and replace the StrLookup.mapLookup()'s Map with a 
> Map (which is the same as Map, 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)

[jira] [Commented] (TEXT-80) StrLookup API confusing

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/TEXT-80?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=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 lookup2 = StrLookup.systemPropertiesLookup();
> StrLookup 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  type parameter 
> everywhere, and replace the StrLookup.mapLookup()'s Map with a 
> Map (which is the same as Map, 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)

[jira] [Commented] (TEXT-80) StrLookup API confusing

[ASF GitHub Bot (JIRA)]

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

ASF GitHub Bot commented on TEXT-80:


Github user garydgregory commented on the issue:

https://github.com/apache/commons-text/pull/44
  
BTW, this will break source compatibility for certain.


> 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 lookup2 = StrLookup.systemPropertiesLookup();
> StrLookup 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  type parameter 
> everywhere, and replace the StrLookup.mapLookup()'s Map with a 
> Map (which is the same as Map, 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)

[jira] [Commented] (TEXT-80) StrLookup API confusing

[ASF GitHub Bot (JIRA)]

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

ASF GitHub Bot commented on TEXT-80:


Github user garydgregory commented on the issue:

https://github.com/apache/commons-text/pull/44
  
Hi All,

Why is it confusing? Why is the patch better? You get the idea, you have to 
make a point that what you propose is better beyond changing source.

Gary


> 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 lookup2 = StrLookup.systemPropertiesLookup();
> StrLookup 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  type parameter 
> everywhere, and replace the StrLookup.mapLookup()'s Map with a 
> Map (which is the same as Map, 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)

[jira] [Commented] (TEXT-80) StrLookup API confusing

[ASF GitHub Bot (JIRA)]

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

ASF GitHub Bot commented on TEXT-80:


Github user chtompki commented on the issue:

https://github.com/apache/commons-text/pull/44
  
Would this imply a 2.0 release because of BC compatibility?


> 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 lookup2 = StrLookup.systemPropertiesLookup();
> StrLookup 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  type parameter 
> everywhere, and replace the StrLookup.mapLookup()'s Map with a 
> Map (which is the same as Map, 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)

[jira] [Commented] (TEXT-80) StrLookup API confusing

[ASF GitHub Bot (JIRA)]

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

ASF GitHub Bot commented on TEXT-80:


Github user britter commented on the issue:

https://github.com/apache/commons-text/pull/44
  
LGTM, @chtompki what do you think?


> 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 lookup2 = StrLookup.systemPropertiesLookup();
> StrLookup 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  type parameter 
> everywhere, and replace the StrLookup.mapLookup()'s Map with a 
> Map (which is the same as Map, 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)

[jira] [Commented] (TEXT-80) StrLookup API confusing

[ASF GitHub Bot (JIRA)]

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

ASF GitHub Bot commented on TEXT-80:


Github user ameyjadiye commented on the issue:

https://github.com/apache/commons-text/pull/44
  
@britter , please review and accept this PR. Travis build on this is 
because previous commit in master , by merging this won't make any issue in 
master. Tested on local before creating PR.

With this comment we can test if activity goes to dev ml.


> 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 lookup2 = StrLookup.systemPropertiesLookup();
> StrLookup 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  type parameter 
> everywhere, and replace the StrLookup.mapLookup()'s Map with a 
> Map (which is the same as Map, 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)

[jira] [Commented] (TEXT-80) StrLookup API confusing

[Pascal Schumacher (JIRA)]

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

Pascal Schumacher commented on TEXT-80:
---

Issue moved from commons-lang to commons-text, because StrLookup was ported to 
commons-text and and the commons-lang version will soon be deprecated.

> StrLookup API confusing
> ---
>
> Key: TEXT-80
> URL: https://issues.apache.org/jira/browse/TEXT-80
> Project: Commons Text
>  Issue Type: Bug
>Reporter: Etienne Neveu
>
> [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 lookup2 = StrLookup.systemPropertiesLookup();
> StrLookup 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  type parameter 
> everywhere, and replace the StrLookup.mapLookup()'s Map with a 
> Map (which is the same as Map, 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)