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

Add functions to check blank Strings in com.google.common.base.Strings #352

Closed
gissuebot opened this issue Oct 31, 2014 · 46 comments
Closed
Labels
status=working-as-intended type=enhancement Make an existing feature better

Comments

@gissuebot
Copy link

Original issue created by jbaumgarten on 2010-04-24 at 07:25 PM


Would be useful to have functions to check blank Strings "à la" commons-lang in
com.google.common.base.Strings.

Could be :
static public boolean isBlank(final String s)
static public boolean isNullOrBlank(@Nullable final String s)

Could also add :
static public @Nullable String blankToNull(@Nullable final String s).

@gissuebot
Copy link
Author

gissuebot commented Oct 31, 2014

Original comment posted by j...@nwsnet.de on 2010-04-27 at 02:05 PM


I second that request. StringUtils.isBlank() is definitely the method in Commons
Lang we by far use most.

The name is also pleasantly compact. I also suggest to rename
Strings.isNullOrEmpty() to Strings.isEmpty() because that is also nicely compact,
uses the same naming style, and eases the transition from Commons Lang to Guava
(however, that last point is of no importance).

@gissuebot
Copy link
Author

Original comment posted by david.henderson on 2010-04-28 at 02:42 PM


With blankToNull, I'd propose a corresponding:

static public String nullToBlank(@Nullable final String s)

@gissuebot
Copy link
Author

Original comment posted by cgdecker on 2010-04-28 at 05:16 PM


What would nullToBlank be for? It'd just be the same as nullToEmpty.

@gissuebot
Copy link
Author

gissuebot commented Oct 31, 2014

Original comment posted by j...@nwsnet.de on 2010-06-21 at 03:09 PM


Any news regarding this?

I just noticed that I can't port my code over from Commons Lang without isNullOrBlank(). It even took me a while to discover that my tests failed because I mistakenly replaced StringUtils.isBlank() with Strings.isNullOrEmpty(). :(

@gissuebot
Copy link
Author

Original comment posted by kevinb@google.com on 2010-07-30 at 03:22 PM


Issue #387 has been merged into this issue.

@gissuebot
Copy link
Author

Original comment posted by neveue on 2010-09-14 at 08:50 AM


I just noticed that I can't port my code over from Commons Lang without isNullOrBlank().

If it were the only thing stopping me from switching to Guava, I would:

  • create a "com.mycompany.common.thingsIWishWereInGuava.Strings2" class
  • copy over the static method I am interested in (isNullOrBlank)
  • configure Eclipse to use static imports as described here: http://piotrjagielski.com/blog/working-with-static-imports-in-eclipse/
  • use the method as if it were in Guava
  • star this issue, to get notified when Guava adds this method (if deemed useful enough), and remove my temporary class

@gissuebot
Copy link
Author

gissuebot commented Oct 31, 2014

Original comment posted by j...@nwsnet.de on 2010-09-14 at 01:41 PM


It's not about switching to Guava (which I did), but ditching (our most common reason for using) Commons Lang.

Introducing a temporarily method, bothering co-workers to use it instead of what they know and have used for years, and then bothering them again to use the Guava method as soon as it hopefully has appeared there is far too cumbersome.

AFAIK, IntelliJ IDEA doesn't support static import favorites as Eclipse do, so that won't work. If it does, please let me know; I badly miss this for unit tests.

@gissuebot
Copy link
Author

Original comment posted by neveue on 2010-09-14 at 03:25 PM


My bad, I did not think about the "social" aspect of such a refactoring. Changing habits is often the hardest part.

In that case, if you really want to get rid of commons-lang, you could simply name your "temporary" utility class "org.apache.commons.lang.StringUtils" (exact same name as commons-lang's StringUtils), and remove the commons-lang jar from your classpath...
But you must be sure commons-lang won't be included by some other third-party jar (such as Spring), or you might have weird classpath errors. But then, if the jar is included anyway, why not keep using the old method for now?
Personally, I wouldn't bother...

Anyway, I was suggesting this mainly as a temporary workaround, in case you did not want to wait for a Guava inclusion to get rid of commons-lang.

I'm surprised that static import favorites are not supported in IntelliJ IDEA (according to http://youtrack.jetbrains.net/issue/IDEABKL-5119 ).

PS: StringUtils.isBlank() and StringUtils.isNotBlank() were also some of the most used commons-lang methods in my last project. Including those helper methods in Guava would be great.

@gissuebot
Copy link
Author

Original comment posted by kevinb@google.com on 2010-09-15 at 05:27 PM


(Note that it is not assured that this method will never appear in Guava.)

@gissuebot
Copy link
Author

Original comment posted by boppenheim@google.com on 2010-09-23 at 05:05 AM


Issue #397 has been merged into this issue.

@gissuebot
Copy link
Author

Original comment posted by bernd.farka on 2010-12-16 at 09:22 PM


is there anything new about this issue? Will a

static public boolean isBlank(final String s)
or
static public boolean isNullOrBlank(@Nullable final String s)

be implemented in further releases?

wbr

@gissuebot
Copy link
Author

Original comment posted by kurt.kluever on 2010-12-17 at 12:22 AM


static public boolean isNullOrBlank(@Nullable final String s)
  >> Strings.isNullOrEmpty() already exists.

static public boolean isBlank(final String s)
  >> If you use Java6, you can just use str.isEmpty()

@gissuebot
Copy link
Author

Original comment posted by cgdecker on 2010-12-17 at 12:46 AM


@kurt.kluever:
Empty and blank are not the same thing... here blank means empty or consisting only of whitespace.

@gissuebot
Copy link
Author

Original comment posted by kurt.kluever on 2010-12-17 at 03:05 AM


That makes this issue really tricky because there are tons of different definitions of whitespace.

@gissuebot
Copy link
Author

Original comment posted by christophe.domas on 2010-12-17 at 07:55 AM


If you use Java6, you can just use str.isEmpty()
Yeap but I first have to check that the str is not null and Strings.isNullOrEmpty() doesn't trim the string.
I'm agree with preceding comments, I really miss this function to remove commons-lang.

@gissuebot
Copy link
Author

Original comment posted by ogregoire on 2010-12-17 at 09:32 AM


@kurt.kluever:
I don't see many definitions of whitespace: Character.isWhitespace(char/int) is the only one in Java.

@gissuebot
Copy link
Author

Original comment posted by kurt.kluever on 2010-12-17 at 03:47 PM


@ogregoire
Pattern.compile("\s"), StringTokenizer, String.trim(), Character.isWhitespace(), Unicode 5.0 "whitespace", and Unicode 5.0 "pattern whitespace" all have different definitions of which characters count as whitespace. Kevin has an awesome (Google internal) spreedsheet breaking down the differences between all of them. I'll ask him if he can post it externally in some form.

@gissuebot
Copy link
Author

Original comment posted by cgdecker on 2010-12-17 at 04:25 PM


@kurt.kluever: I'm guessing you're talking about this spreadsheet that's linked from CharMatcher.WHITESPACE: http://spreadsheets.google.com/pub?key=pd8dAQyHbdewRsnE5x5GzKQ

I agree that the different definitions of whitespace is an issue here. I think what many people would be replacing here is either "string == null || string.trim().length() == 0" (which uses the trim definition obviously) or StringUtils.isBlank, which uses Character.isWhitespace.

It's not as pretty as "isNullOrBlank", but maybe a more flexible approach would be something like:

   boolean isNullOrMatches(String string, CharMatcher matcher);

That would allow users to choose their definition of whitespace:

   if (isNullOrMatches(string, CharMatcher.JAVA_WHITESPACE)) { ... }

I feel like the problem then is that people don't want to have to choose and are happy with having one of Java's definitions of whitespace chosen for them. Maybe that's a bad thing.

@gissuebot
Copy link
Author

Original comment posted by neveue on 2010-12-17 at 04:42 PM


Looking at CharMatcher also reveals a number of whitespace definitions, such as CharMatcher.WHITESPACE, CharMatcher.BREAKING_WHITESPACE and CharMatcher.JAVA_WHITESPACE.

CharMatcher.WHITESPACE.matchesAllOf(s) does the same as the isBlank(s) method suggested above (it is more verbose, though). If one needs to check the "blankness" according to a different kind of "whitespaceness", one could use the other CharMatchers.
It wouldn't replace isNullOrBlank(), because matchesAllOf() rejects null values (one of Guava's principles is to reject null values, which is a most of the time a good thing).

I'd like to think about the bigger picture: simple methods such as isBlank() and isNullOrBlank() would be great, but it would be even better to be able to combine Predicate, CharMatcher, and string matching. Note that you can already do that by creating utility classes such as:

public static StringPredicates {

public static final Predicate<String> IS_BLANK = matchesAllOf(CharMatcher.WHITESPACE);

public static final Predicate<String> IS_NULL_OR_BLANK = Predicates.and(Predicates.notNull(), IS_BLANK);

public static Predicate<String> matchesAllOf(final CharMatcher charMatcher) {
    return new Predicate<String>() {
        public boolean apply(String string) {
            return charMatcher.matchesAllOf(string);
        }
    };
}

}

Then, in your code, using static imports:

public void foo(String maybeBlankOrNull) {
    if (IS_NULL_OR_BLANK.apply(maybeBlankOrNull)) {
        // do something
    }
}

public void foo(List<String> stringsMaybeBlankOrNull) {
    List<String> strings = ImmutableList.copyOf(Collections2.filter(stringsMaybeBlankOrNull, Predicates.not(IS_NULL_OR_BLANK));
    // do something

}

Since predicates might sometimes be less readable, we could also add some of these utility methods with sensible defaults ("isNullOrBlank(s)" is nicer to read than "IS_NULL_OR_BLANK.apply(s)").

PS: I saw Colin's post after writing this... I think some of our ideas are similar

@gissuebot
Copy link
Author

Original comment posted by cody.lerum on 2010-12-31 at 08:46 PM


what about having an option to trim on the isNullOrEmpty. That gives people the option to decide if whitespace is considered empty or not for them.

boolean isNullOrEmpty(String string, boolean trim)
{
   if(trim)
   {
      return string == null || string.trim().length() == 0;
   }
   else
   {
      return string == null || string.length() == 0;
   }
}

boolean isNullOrEmpty(String string)
{
   return isNullOrEmpty(string, false);
}

@gissuebot
Copy link
Author

Original comment posted by lystochok on 2011-01-01 at 12:18 PM


cody.lerum,
This is what isNullOrBlank() should actually do, boolean parameters generally cause confusion.

@gissuebot
Copy link
Author

Original comment posted by cody.lerum on 2011-01-03 at 01:58 AM


I'm fine with isNullOrBlank(). What is the status of this making it into a future guava?

@gissuebot
Copy link
Author

Original comment posted by fry@google.com on 2011-01-28 at 04:05 PM


(No comment entered for this change.)


Status: Accepted
Labels: Type-Enhancement

@gissuebot
Copy link
Author

Original comment posted by kevinb@google.com on 2011-02-03 at 06:32 AM


We have had internal discussion about it. I have begged for someone who understands why this method would actually be useful to explain it to me, because I just do not get it.

Why would you ever want to know if a string is "blank", as you say, but NOT want to actually trim the thing? The fact you're even wanting to call isBlank() proves that leading and trailing whitespace is insignificant to you, so why on earth leave it there?

Where you say "checkArgument(!Strings.isBlank(s)); this.s = s;" I see "this.s = WHITESPACE.trimFrom(s); checkArgument(!s.isEmpty());". The latter makes sense to me, the first doesn't.

And even at that, for most of the usages of this kind of method that I look at, I can't even find any good reason why a string with whitespace around it should be accepted in the first place. I mean, sure, in a UI, where a user is entering text, of course you want to get whitespace off of that, but I see this isBlank() nonsense everywhere at all levels -- why? Who's going to be sticking whitespace around everything? Of course, I strongly suspect that there is no need to accept this kind of messy data, but well-intentioned developers think that it's always nice to be as accepting as possible (which is dead wrong).

So. What am I missing? It must be something, because this seems to be the most popular method in the history of computing.

@gissuebot
Copy link
Author

gissuebot commented Oct 31, 2014

Original comment posted by j...@nwsnet.de on 2011-02-03 at 09:37 AM


You are right in that one would really want to trim the whitespace before using (and checking!) in most cases.

One could think of cases, though, in which user input should be considered missing when nothing or just whitespace was given, but when letters were given, no leading or trailing whitespace should be trimmed (because it matters, e.g. in various markup languages).

Another issue I see that people want both a boolean indicating of a string is "blank" as well as the string itself (trimmed or not). That's hard to do with a single method as the result must be used both in a condition and for further string processing.

While WHITESPACE.trimFrom(s) solves the issue of s.trim() (where s could be null and thus needs to be checked separately), it does require a static import (or is quite long when called with the containing class' name).

I'll keep an eye on our usages, maybe one could all do it differently. This might be a case of something that is just used routinely without actually re-evaluating it.

In the end, a fitting helper method should be doable, but it might not be isBlank (as I, too, was thinking of out of habit).

@gissuebot
Copy link
Author

Original comment posted by kevinb@google.com on 2011-02-03 at 02:12 PM


"One could think of cases...." I don't know who this "one" is, but it's not me. :)

I don't blindly accept that "null needs to be checked separately." Most of the time, null should simply cause NullPointerException.

We created CharMatcher in full awareness that using it will be more verbose than if we had a set of 47 static methods instead. It's worth it. It exposes the flexibility to you that you're going to need if not here, then somewhere else. Also, perhaps it prompts you to question whether our WHITESPACE constant is really the right definition of whitespace that you're looking for... since there are so many.

@gissuebot
Copy link
Author

Original comment posted by kevinb@google.com on 2011-02-03 at 03:40 PM


Actually, after further internal discussion, I'm more convinced than ever that this is not a useful method. I'm closing it. Well-reasoned arguments with strong evidence will still be listened to.


Status: WorkingAsIntended

@gissuebot
Copy link
Author

Original comment posted by chris.lercher on 2011-06-09 at 03:24 PM


@kevinb: I don't think that it's always the case that you need a trimmed version of the string after the test. I agree with you: I wouldn't use it in such cases. But I've had reasons to use isBlank() lots of times, like in this simple example:

// Example 1

StringBuffer sb = new StringBuffer();
...
if (StringUtils.isBlank(email)) {
    sb.append("[not entered]"); // I don't need email trimmed!
}
...

And there are complex examples:
(this is just an arbitrary, real-life example I'm currently encountering in my code)

// Example 2

Google ReCaptcha explains (http://code.google.com/apis/recaptcha/docs/tips.html), that you should avoid sending captcha requests to Google's service, if the user's captcha_response_field was empty (let's say: blank). So I test it with StringUtils.isBlank(captchaResponseField);

However: I do want to send the captcha_response_field unmodified and untrimmed, in case the user entered at least one non-whitespace character. (Probably I could trim it, but I feel that it's more clean to send it unmodified, because sometimes ReCaptcha accidentally displays one blank word (never two blank words). Who knows, if the service might work better, if I send it untrimmed? I don't know. But my point is: The isBlank test can be seen as separate from the need to trim a string.)

Note: The value can easily be null (if the POST request from the user didn't contain the captcha_response_field parameter at all).

StringUtils.isBlank() is very useful, and often makes the code so much more readable.

@gissuebot
Copy link
Author

Original comment posted by kurt.kluever on 2011-06-09 at 06:33 PM


StringBuffer sb = new StringBuffer();
if (StringUtils.isBlank(email)) {
sb.append("[not entered]"); // I don't need email trimmed!
}

How about a 1 liner:
sb.append(Objects.firstNonNull(Strings.emptyToNull(email), "[not entered]"));

@gissuebot
Copy link
Author

Original comment posted by chris.lercher on 2011-06-09 at 06:59 PM


@kurt: Unfortunately, that's not the same: Note the difference between "isBlank()" and "isEmpty()" - see http://commons.apache.org/lang/api-release/org/apache/commons/lang/StringUtils.html#isBlank(java.lang.String)

i.e. in your example, we would need a Strings.blankToNull() method

@gissuebot
Copy link
Author

Original comment posted by gscerbak on 2011-06-09 at 07:39 PM


Just to sum up:

  1. Blank is null, empty, or containing only whitespace:
    isBlank(null) == true
    isBlank("") == true
    isBlank(" ") == true
    isBlank(" ") == true
    isBlank(" x") == false

  2. There isn't just one definition of what whitespace is.

  3. WHITESPACE.matchesAllOf(nullToEmpty(email))

  4. There are IMHO far more interesting one liners that were rejected in Guava.

@gissuebot
Copy link
Author

Original comment posted by kurt.kluever on 2011-07-07 at 02:46 PM


Issue #652 has been merged into this issue.

@gissuebot
Copy link
Author

Original comment posted by elchin.asgarli on 2011-07-07 at 03:34 PM


What about passing a special Enum(WhitespaceTypeEnum) as a second argument to isNullOrBlank, which would give the definition of whitespace?

Why would you ever want to know if a string is "blank", as you say, but NOT want to actually trim the thing? The fact you're even wanting to call isBlank() proves that leading and trailing whitespace is insignificant to you, so why on earth leave it there?

Because I don't want to be checking for null before calling trim(), that would spoil the whole point of using a library. The beauty of isNullOrEmpty is that it checks simultaneously(!) for null and emptiness.

@gissuebot
Copy link
Author

Original comment posted by raymond.rishty on 2011-07-07 at 04:56 PM


Well you can't safely trim a string if you've not yet checked for null. If you have checked for null, you probably wouldn't be using isNullOrEmpty either

@gissuebot
Copy link
Author

Original comment posted by chris.lercher on 2011-07-07 at 05:31 PM


@raymond: Consider this example:

if (!StringUtils.isBlank(str)) {

   stringBuffer.append(str.trim());
}

@gissuebot
Copy link
Author

Original comment posted by amertum on 2011-07-07 at 06:54 PM


better like this :
stringBuffer.append(Strings.nullToEmpty(str).trim());

@gissuebot
Copy link
Author

Original comment posted by elchin.asgarli on 2011-07-07 at 07:23 PM


@lercherc
And what if in that case str is something like " ", and according to my business logic I should have processed is same as null or empty String? In this particular case with StringBuffer that will not make a difference, however if you want to pass it to for example DataFormat.parse() or Integer.parseInt(), then it does not work.

@gissuebot
Copy link
Author

Original comment posted by chris.lercher on 2011-07-07 at 07:34 PM


@elchin: My example (using isBlank) works also in your case. The modified example by amer...@gmail.com (using nullToEmpty) will only work for some situations.

So let's make a better example:

if (!StringUtils.isBlank(str)) {

stringBuffer.append("[not entered]");

} else {

stringBuffer.append(str.trim());

}

@gissuebot
Copy link
Author

Original comment posted by elchin.asgarli on 2011-07-07 at 07:43 PM


@lercherc
I have a bit of a hard time trying to figure out your example, however I am going to show you mine, and I want to see if there is currently a way to do it:

try {
    if (StringUtils.isNullOrBlank(inputString)) {
        // do nothing, optional date field left empty
    } else {
        someBusinessObject.setSomeBusinessDate(someDateFormat.parse(inputString));
    }
} catch (ParseException e) {
    throw new IllegalArgumentException("Date given in wrong format");
}

@gissuebot
Copy link
Author

Original comment posted by chris.lercher on 2011-07-07 at 07:46 PM


@elchin: Yes, that's basically a very similar example. It adds another valid use case.

@gissuebot
Copy link
Author

Original comment posted by amertum on 2011-07-07 at 08:06 PM


Usefulness of trailing spaces is very rare, again check the comment of kevinb #352

If your if-statement does nothing why keeping it ?

@gissuebot
Copy link
Author

Original comment posted by elchin.asgarli on 2011-07-07 at 08:11 PM


Because otherwise (in case of blank String) someDateFormat.parse(inputString) will throw ParseException!

@gissuebot
Copy link
Author

gissuebot commented Oct 31, 2014

Original comment posted by j...@nwsnet.de on 2011-07-08 at 09:11 AM


I wonder if a wrapper method around String.trim(String) which would return Optional<String> if the input value is null or - after trimming - an empty string would be helpful here.

It could be used like this (based on the example in comment #39):

  stringBuilder.append(Strings.trim(input).or("[not entered]"));

@gissuebot
Copy link
Author

gissuebot commented Oct 31, 2014

Original comment posted by j...@nwsnet.de on 2011-07-08 at 09:30 AM


Addenda:

The method would of course return Optional<String> in any case, that'd be part of the signature.

Also, the definition of whitespace is the one used by String.trim() (though it is different from StringUtils.isBlank(), which uses Characters.isWhitespace()) and thus developers should be at least familiar with its behaviour.

@gissuebot
Copy link
Author

Original comment posted by cgdecker@google.com on 2014-05-21 at 06:48 PM


Issue #1760 has been merged into this issue.

@gissuebot
Copy link
Author

Original comment posted by cgdecker@google.com on 2014-05-21 at 06:48 PM


Issue #1760 has been merged into this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status=working-as-intended type=enhancement Make an existing feature better
Projects
None yet
Development

No branches or pull requests

1 participant