[GitHub] MarkDacek commented on issue #395: Replaces the given String, with the String which is nested in between two Strings.

classic Classic list List threaded Threaded
1 message Options
Reply | Threaded
Open this post in threaded view
|

[GitHub] MarkDacek commented on issue #395: Replaces the given String, with the String which is nested in between two Strings.

GitBox
MarkDacek commented on issue #395: Replaces the given String, with the String which is nested in between two Strings.
URL: https://github.com/apache/commons-lang/pull/395#issuecomment-453863749
 
 
   For one, the default return value should be the original `str`, not null. That way, it would be much simpler to use it safely, e.g.:
   `list.add(StringUtils.replaceSubstringInBetween(args..))`
   instead of having to add in a null check before adding it.
   
   Please add sufficient unit-tests to show that this functions as expected.
   
   I would also suggest that a boolean `repeat` be added, to allow this to be done indefinitely. This might require a slight refactor.
   
   Other comments to be made inline.
   
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[hidden email]


With regards,
Apache Git Services