CODEWITHSUNDEEP
javacoding-stylecontains
Bohemian
Bohemian

Reputation: 424953

String contains vs List<String> contains

Assuming you want to test if an input is one of several constant Strings, and ignoring performance, is it an anti-pattern to code:

if ("yes oui ja da".contains(answer)) {
    // answer was in the affirmative
}

instead of the more conventional:

private static List<String> affirmativeAnswers = Arrays.asList("yes", "oui", "ja", "da");

if (affirmativeAnswers.contains(answer)) {
    // answer was in the affirmative
}

It's a lot less code and easier to read, but is it a "hack"?

Edit:

For more safety, if you are worried about partial matches, you can code it as:

if (",yes,oui,ja,da,".contains(',' + answer + ','))

It's still much less code (although getting ugly)

Upvotes: 3

Views: 1643

Answers (5)

Stephen C
Stephen C

Reputation: 718678

... is it an abuse of the language ...

"Java - he don't care!!". But "Eww!! Nasty!!"1.

But seriously, you should be aiming to make your code easy to read and easy to maintain. (Or performant ... if that matters.)

Tricky, obscure code that expresses something using the minimum number of keystrokes is none of the above. It is bad style ... even if you get the code functionally correct.

1 - if you've been living under a rock for the past year or so ... I'm alluding to this - http://knowyourmeme.com/memes/honey-badger.

Upvotes: 0

Yogesh Patil
Yogesh Patil

Reputation: 888

If you want to check for the presence of the string in the sentence, then
string.contains(string) is feasible.

Else, if you want to check for the equality of whole strings, then,
list.contains(string) is feasible.

Upvotes: 1

Travis J
Travis J

Reputation: 82267

Remove dependencies where possible

In my opinion it is not best practice. Best practice would allow for the removal of the dependency on the acceptance strings. In this case, the dependency is this ("yes", "oui", "ja", "da") list of strings. However, if the dependency on the list of strings were to be moved to a service or database, it would be very easy to integrate that into a method which accepted List<string> because it is obvious, whereas issues could arise when trying to integrate into a method which simply expects a string.

Keep scope in mind

As always, this depends on scope. If you only needed to do this small thing one time and it was part of a very small project, then it really makes little difference. However, if this is a piece of a project which is not small, then it could require refactoring down the road if best practices were not observed.

Let the compiler optimize the code

In regards to the amount of code it takes, you should not worry too much about that. It is more important that it is readable. The compiler will make the code as efficient as possible because that is it's duty. Do not attempt to write code similar to a compiler, because as better approaches emerge, the compiler will use them whereas the emulated code will remain the same.

Upvotes: 0

aa8y
aa8y

Reputation: 3942

Also, AFAIK, the String contains method uses regex to check if a substring is a part of a string. And this operation is expensive compared to searching a list (AFAIK). So IMO, it's better to use a list now, isn't it? ;)

Upvotes: 0

mikera
mikera

Reputation: 106351

I would regard this as an ugly hack, for various reasons:

  • It isn't robust to unexpected input. For example ("yes oui ja da".contains(" ")) would return true - probably not what you intended. This is I think the biggest problem. Even if you start adding more tricks (like the commas in the recent edit) then you still have nasty corner cases to consider.
  • It isn't as performant in the general case - if you have a medium/large number of possibilities to test for, you are probably better checking against a HashMap or HashSet which is O(1) as opposed to O(n) for scanning a concatenated String.
  • It might confuse an inexperienced coder/future maintainer. Don't use clever tricks if you want your code to be maintainable.
  • It doesn't lend itself well to future refactoring (e.g. internationalisation of Strings? altering the list of possibilities dynamically at runtime?)

Upvotes: 5

Related Questions