Finding all plurals in an array of Strings

Question

I'm looking for feedback on this here code. Is readability an issue? Is it good to mess with all these data types in Java (e.g. strings and Chars and using .equals() and != to compare data from different data types), or is there an easier way to do it? And in general, how can I improve this code?

Problem Description

Write a method named allPlural that accepts an array of strings as a parameter and returns true only if every string in the array is a plural word, and false otherwise. For this problem a plural word is defined as any string that ends with the letter S, case-insensitively. The empty string "" is not considered a plural word, but the single-letter string "s" or "S" is. Your method should return true if passed an empty array (one with 0 elements).

 public static boolean allPlural(String[] words){
   for (String s : words) {
     // blank string is not plural
     if (s.equals("")) {
       return false;
     }
     // if the strings in the array don't have an s on the end, return false
     if (Character.toUpperCase(s.charAt(s.length()-1)) != 'S') {
       return false;
     }
   }
   return true;
 }

• 200_success: Please transcribe the screenshot.
• Eric Lippert: Consider breaking it down into two methods: one, a function that takes a string and returns a bool, and two, a function that takes an array and a predicate, and returns false if the predicate is false for any member of the array, and true otherwise. Once you have those two general-purpose tools you can easily combine them together.

Answer 1

This code is pretty good. I definitely recommend extracting another function isPlural:

private static boolean isPlural(String word) {
 // blank string is not plural
 if (word.equals("")) {
 return false;
 }
 // if the strings in the array don't have an s on the end, return false
 if (Character.toUpperCase(word.charAt(word.length()-1)) != 'S') {
 return false;
 }
 return true;
}

You might notice that this removes the need for the comments. The name of the function makes it clear that if (word.equals("")) { return false; } means that empty strings aren't considered plural. Do note that you should use String#isEmpty as a more descriptive function for what you are doing.

private static boolean isPlural(String word) {
 if (word.isEmpty()) {
 return false;
 }
 if (Character.toUpperCase(word.charAt(word.length()-1)) != 'S') {
 return false;
 }
 return true;
}

You can also remove conditionals to produce code like so:

private static boolean isPlural(String word) {
 if (word.isEmpty()) {
 return false;
 }
 return Character.toUpperCase(word.charAt(word.length() - 1)) == 'S';
}

You could collapse the word.isEmpty() into the return statement, but that comes down to preference:

private static boolean isPlural(String word) {
 return !word.isEmpty()
 && Character.toUpperCase(word.charAt(word.length() - 1)) == 'S';
}

---

Now your function would look like this:

public static boolean allPlural(String[] words){
 for (String s : words) {
 if (!isPlural(s)) return false;
 }
 return true;
}

However, this is a well known algorithm known as "all". The Stream API has a method that does just what we want: Stream#allMatch. Combining it with Arrays#stream we can write the result very concisely:

public static boolean allPlural(String[] words) {
 return Arrays.stream(words).allMatch(ContainingClass::isPlural);
}

Answer 2

The code is well readable as it is. As long as it solves the problem (it does), it is fine.

For a real-world plural detection, I would add to the class:

public static boolean isPlural(String s)

to detect if a single word is a proper plural, because it is more complicated than 'ends with "S"'.

• cody.codes: Good to add if I was to write a class for it. In this case I just did a method, so I went with the below answer as it was a nice refactor to the code.

Answer 3

I think this is more readable and a bit shorter:

public static boolean allPlural(String[] words) {
 for (String s : words) {
 if (!s.endsWith("S") && !s.endsWith("s")) return false;
 }
 return true;
}

This one will be a lot slower due to multiple compilations of the regex, but shorter code:

public static boolean allPlural(String[] words) {
 for (String s : words) {
 if (!s.matches(".*[Ss]")) return false;
 }
 return true;
}

You could pull out the regex like this:

private static Pattern PLURAL_PAT = Pattern.compile(".*[sS]");
public static boolean allPlural(String[] words) {
 for (String s : words) {
 if (!PLURAL_PAT.matcher(s).matches()) return false;
 }
 return true;
}

• CAD97: If you're going to suggest using RegEx, use a better RegEx expression: Pattern.compile("s$", Pattern.CASE_INSENSITIVE). That regex says "contains s followed by the end of the string, case insensitively." (Depending on how good the JVM regex engine is, that regex may even reach near-optimal speed.)
• JimB: CAD97 - nice call on the case insensitive, but the $ won't work as you suggest because matches() compares against the entire string. You could use find() instead of matches() though. Since we're only checking case on a single character, I'm not convinced that case_insensitive is inherently better in this case. Probably all the same after compilation.
• CAD97: Right. I've been working with other languages' regex whose default are more closely aligned with raw regex definition. I really should remember that matches only matches the full string though, I've debugged that exact thing multiple times now...