You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by PascalSchumacher <gi...@git.apache.org> on 2016/11/03 20:15:28 UTC
[GitHub] commons-lang pull request #205: LANG-1281: Javadoc of StringUtils.ordinalInd...
GitHub user PascalSchumacher opened a pull request:
https://github.com/apache/commons-lang/pull/205
LANG-1281: Javadoc of StringUtils.ordinalIndexOf is contradictory
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/PascalSchumacher/commons-lang StringUtils#ordialIndexOf_javadoc
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/commons-lang/pull/205.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #205
----
commit e5997106b568d321462278911980b8c81d0679e6
Author: pascalschumacher <pa...@gmx.net>
Date: 2016-11-03T20:14:36Z
LANG-1281: Javadoc of StringUtils.ordinalIndexOf is contradictory
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] commons-lang issue #205: LANG-1281: Javadoc of StringUtils.ordinalIndexOf is...
Posted by coveralls <gi...@git.apache.org>.
Github user coveralls commented on the issue:
https://github.com/apache/commons-lang/pull/205
[](https://coveralls.io/builds/8673668)
Coverage increased (+0.02%) to 93.58% when pulling **ac05015befda654b2cd6e7a07c2eca5785d49045 on PascalSchumacher:StringUtils#ordialIndexOf_javadoc** into **ff4497aff8cc9de4e0b2c6e5e23e5b6550f76f29 on apache:master**.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] commons-lang pull request #205: LANG-1281: Javadoc of StringUtils.ordinalInd...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/commons-lang/pull/205
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] commons-lang issue #205: LANG-1281: Javadoc of StringUtils.ordinalIndexOf is...
Posted by kinow <gi...@git.apache.org>.
Github user kinow commented on the issue:
https://github.com/apache/commons-lang/pull/205
Oh, now it makes sense @aioobe
I like
"increments by 1 unless the search string is the empty string in which case the position is never incremented"
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] commons-lang issue #205: LANG-1281: Javadoc of StringUtils.ordinalIndexOf is...
Posted by aioobe <gi...@git.apache.org>.
Github user aioobe commented on the issue:
https://github.com/apache/commons-lang/pull/205
Looks good to me, even though I think it's a terrible idea to view empty strings as some form of special strings and treat them differently than other strings.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] commons-lang issue #205: LANG-1281: Javadoc of StringUtils.ordinalIndexOf is...
Posted by PascalSchumacher <gi...@git.apache.org>.
Github user PascalSchumacher commented on the issue:
https://github.com/apache/commons-lang/pull/205
Thanks everybody!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] commons-lang issue #205: LANG-1281: Javadoc of StringUtils.ordinalIndexOf is...
Posted by aioobe <gi...@git.apache.org>.
Github user aioobe commented on the issue:
https://github.com/apache/commons-lang/pull/205
But the implementation of the method does NOT increment by 1 if given the empty string. So this change causes the documentation to contradict the implementation.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] commons-lang issue #205: LANG-1281: Javadoc of StringUtils.ordinalIndexOf is...
Posted by PascalSchumacher <gi...@git.apache.org>.
Github user PascalSchumacher commented on the issue:
https://github.com/apache/commons-lang/pull/205
`...incrementing the starting index by one after each successful match, so matches may overlap.` For empty String there is no match, so no incrementing is done.
Please do not hesitate to suggest better solution for this issue. Thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] commons-lang issue #205: LANG-1281: Javadoc of StringUtils.ordinalIndexOf is...
Posted by aioobe <gi...@git.apache.org>.
Github user aioobe commented on the issue:
https://github.com/apache/commons-lang/pull/205
Of course there are matches for the empty string. a) there is both a leading and trailing empty string in "a" for instance. b) the implementation doesn't return INDEX_NOT_FOUND when given the empty string.
As far as I can tell, the implementation increments the position with 1 each time a match is found, except if the sought string is the empty string, in which case it increments with 0 (conceptually that is; in practice it obviously returns 0 right away for that case, since if it increments by 0 each match, it will never move away from 0).
Personally I think the implementation is messy and hard to formalize. It would have been much better if the implementation consistently stuck the idea of incrementing by 1 for each match. This would not rule out the optimization because one would simply return n for the empty search string case.
Given the situation I guess it's not an option to change the implementation though, so the only option left is to adjust the documentation to reflect the current behavior. Here are two suggestions:
"increments by 1 unless the search string is the empty string in which case the position is never incremented"
or something like
"always returns 0 if the search string is the empty string, otherwise it increments the position by 1 for each match found"
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] commons-lang issue #205: LANG-1281: Javadoc of StringUtils.ordinalIndexOf is...
Posted by coveralls <gi...@git.apache.org>.
Github user coveralls commented on the issue:
https://github.com/apache/commons-lang/pull/205
[](https://coveralls.io/builds/8658876)
Coverage increased (+0.007%) to 93.563% when pulling **e5997106b568d321462278911980b8c81d0679e6 on PascalSchumacher:StringUtils#ordialIndexOf_javadoc** into **ff4497aff8cc9de4e0b2c6e5e23e5b6550f76f29 on apache:master**.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] commons-lang issue #205: LANG-1281: Javadoc of StringUtils.ordinalIndexOf is...
Posted by kinow <gi...@git.apache.org>.
Github user kinow commented on the issue:
https://github.com/apache/commons-lang/pull/205
+1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] commons-lang issue #205: LANG-1281: Javadoc of StringUtils.ordinalIndexOf is...
Posted by PascalSchumacher <gi...@git.apache.org>.
Github user PascalSchumacher commented on the issue:
https://github.com/apache/commons-lang/pull/205
I guess we should hide the implementation details and just state `Note: Matches may overlap.`.
What do you think?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] commons-lang issue #205: LANG-1281: Javadoc of StringUtils.ordinalIndexOf is...
Posted by PascalSchumacher <gi...@git.apache.org>.
Github user PascalSchumacher commented on the issue:
https://github.com/apache/commons-lang/pull/205
Updated the pull request using "increments by 1 unless the search string is the empty string in which case the position is never incremented".
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---