Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lang-1345 Enhance non-empty strings #278

Open
wants to merge 4 commits into
base: master
from

Conversation

@ThrawnCA
Copy link

commented Jul 17, 2017

No description provided.

@coveralls

This comment has been minimized.

Copy link

commented Jul 17, 2017

Coverage Status

Coverage increased (+0.001%) to 95.254% when pulling f1ef48b on ThrawnCA:LANG-1345-enhance-non-empty-strings into 69e8438 on apache:master.

* @see StringUtils#defaultString(String, String)
*/
public static String extendIfNotBlank(final String str, final String prefix, final String suffix) {
return isBlank(str) ? "" : defaultString(prefix) + str + defaultString(suffix);

This comment has been minimized.

Copy link
@garydgregory

garydgregory Jan 12, 2018

Contributor

Reuse the EMPTY constant.

* @see StringUtils#defaultString(String, String)
*/
public static String extendIfNotEmpty(final String str, final String prefix, final String suffix) {
return isEmpty(str) ? "" : defaultString(prefix) + str + defaultString(suffix);

This comment has been minimized.

Copy link
@garydgregory

garydgregory Jan 12, 2018

Contributor

Reuse the EMPTY constant.

Copy link
Contributor

left a comment

I really do not like the method names starting with "extend". It does not smell right to me. We have some methods here that start with "default", maybe that's better.

@stokito

This comment has been minimized.

Copy link

commented Jun 23, 2018

For me is not clear importance of the new methods. I can't remember situations when I needed for something like this. And even if I need something like this I'll write the function myself and I'll not even think about to search something in Commons Lang library. Time spent to looking for the method will be ten time more then write myself.
Maybe commons lang itself uses the function internally then it can make sense.

It would be great if you can give us an examples of usage and how they are important.

@ThrawnCA

This comment has been minimized.

Copy link
Author

commented Jun 24, 2018

A typical case would be that you're assembling a sentence, and if a component is not empty, you need to add a space beforehand. (Sometimes join will handle this, but other times it's not the right tool for the situation.)

Or you're building a URL, and if there's a non-blank query string, you need to put a question mark before it.

Really, any situation where part of a string is optional but requires special formatting if present.

@garydgregory The trouble with "default" names is that this functionality is the opposite. The defaultXXX methods have the logic, "if present, leave untouched; otherwise use the second parameter." This would instead be, "if present, apply the second parameter; otherwise, leave untouched." Could use "appendIfNotXXX" and "prependIfNotXXX" I suppose, but I'd rather combine prefixes and suffixes in one method.

@aaabramov

This comment has been minimized.

Copy link
Contributor

commented Sep 26, 2018

Could use "appendIfNotXXX" and "prependIfNotXXX" I suppose, but I'd rather combine prefixes and suffixes in one method.

I think this naming would be better and more granular.

@ThrawnCA

This comment has been minimized.

Copy link
Author

commented Nov 1, 2018

And I've come across another situation where I want this: assembling a filename from customer name parts, and adding a separator for middle name if (and only if) it's provided.

* <p>Whitespace is defined by {@link Character#isWhitespace(char)}.</p>
*
* <pre>
* StringUtils.appendIfNotBlank(null, " ") = ""

This comment has been minimized.

Copy link
@RahulNagekar

RahulNagekar Dec 1, 2018

Contributor

The javadoc needs an update to be in sync with the method name.

* StringUtils.appendIfNotBlank(null, " ") = ""
* StringUtils.appendIfNotBlank(null, "-post") = ""
* StringUtils.appendIfNotBlank("", "-post") = ""
* StringUtils.appendIfNotBlank(" ", " ") = ""

This comment has been minimized.

Copy link
@RahulNagekar

RahulNagekar Dec 1, 2018

Contributor

9050 : The method behavior doesn't seem to be in sync with its name for this test case.
Current behaviour :
StringUtils.appendIfNotBlank(" ", "-post") = ""
Behavior as per the name:
StringUtils.appendIfNotBlank(" ", "-post") = " "

May be should preserve the original string although if its blank.

* <p>Whitespace is defined by {@link Character#isWhitespace(char)}.</p>
*
* <pre>
* StringUtils.appendIfNotBlank(null, " ") = ""

This comment has been minimized.

Copy link
@RahulNagekar

RahulNagekar Dec 1, 2018

Contributor

Shouldn't this be returning null instead as the name doesn't say anything about making the original string empty?

* <p>Whitespace is defined by {@link Character#isWhitespace(char)}.</p>
*
* <pre>
* StringUtils.prependIfNotEmpty(null, " ") = ""

This comment has been minimized.

Copy link
@RahulNagekar

RahulNagekar Dec 1, 2018

Contributor

Shouldn't this be returning null instead as the name doesn't say anything about making the original string empty?

* <p>Whitespace is defined by {@link Character#isWhitespace(char)}.</p>
*
* <pre>
* StringUtils.prependIfNotBlank(null, " ") = ""

This comment has been minimized.

Copy link
@RahulNagekar

RahulNagekar Dec 1, 2018

Contributor

Shouldn't this be returning null instead as the name doesn't say anything about making the original string empty?

* @return the passed in String with suffix added, or empty string
*/
public static String appendIfNotEmpty(final String str, final String suffix) {
return isEmpty(str) ? EMPTY : str + defaultString(suffix);

This comment has been minimized.

Copy link
@RahulNagekar

RahulNagekar Dec 1, 2018

Contributor
Suggested change
return isEmpty(str) ? EMPTY : str + defaultString(suffix);
return isEmpty(str) ? str : str + defaultString(suffix);
* @return the passed in String with suffix added, or empty string
*/
public static String appendIfNotBlank(final String str, final String suffix) {
return isBlank(str) ? EMPTY : str + defaultString(suffix);

This comment has been minimized.

Copy link
@RahulNagekar

RahulNagekar Dec 1, 2018

Contributor
Suggested change
return isBlank(str) ? EMPTY : str + defaultString(suffix);
return isBlank(str) ? str : str + defaultString(suffix);
* @return the passed in String with prefix added, or empty string
*/
public static String prependIfNotEmpty(final String str, final String prefix) {
return isEmpty(str) ? EMPTY : defaultString(prefix) + str;

This comment has been minimized.

Copy link
@RahulNagekar

RahulNagekar Dec 1, 2018

Contributor
Suggested change
return isEmpty(str) ? EMPTY : defaultString(prefix) + str;
return isEmpty(str) ? str : defaultString(prefix) + str;
* @return the passed in String with prefix added, or empty string
*/
public static String prependIfNotBlank(final String str, final String prefix) {
return isBlank(str) ? EMPTY : defaultString(prefix) + str;

This comment has been minimized.

Copy link
@RahulNagekar

RahulNagekar Dec 1, 2018

Contributor
Suggested change
return isBlank(str) ? EMPTY : defaultString(prefix) + str;
return isBlank(str) ? str : defaultString(prefix) + str;
* StringUtils.appendIfNotBlank(" ", "-post") = ""
* StringUtils.appendIfNotBlank("abc", null) = "abc"
* StringUtils.appendIfNotBlank("abc", "") = "abc"
* StringUtils.appendIfNotBlank("abc", " ") = "abc"

This comment has been minimized.

Copy link
@RahulNagekar

RahulNagekar Dec 1, 2018

Contributor

Needs correction

Suggested change
* StringUtils.appendIfNotBlank("abc", " ") = "abc"
* StringUtils.appendIfNotBlank("abc", " ") = "abc "
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.