Cleaner code suggestion for string manipulation

Question

So I thought a lot about the logic of my if statement on line 6 of my code. But I was hoping to get some feedback from more experienced developers. You guys think I just messed up my code, and if so, how would you write it more concise?

Answer 1

Over-complication? Yes, you really :-)

I would just use a lot easier:

public String seeColor (String color) { if (color.startsWith("red")) return "red"; if (color.startsWith("blue")) return "blue"; return ""; } 

The following complete program shows this in action:

 public class Test { public static String seeColor (String color) { if (color.startsWith("red")) return "red"; if (color.startsWith("blue")) return "blue"; return ""; } public static void main(String[] args) { String[] testData = { "redxx", "xxred", "blueTimes", "NoColor", "red", "re", "blu", "blue", "a", "", "xyzred" }; for (String s: testData) System.out.println("[" + s + "] -> [" + seeColor(s) + "]"); } } 

The output of this program, as expected:

 [redxx] -> [red] [xxred] -> [] [blueTimes] -> [blue] [NoColor] -> [] [red] -> [red] [re] -> [] [blu] -> [] [blue] -> [blue] [a] -> [] [] -> [] [xyzred] -> [] 

---

If you want it to be easily extensible in the future, you could choose something like:

 public static String seeColor (String color) { String[] allow = {"red", "blue"}; for (String s: allow) if (color.startsWith(s)) return s; return ""; } 

Then adding color to the allow array is the only step you need to take to recognize it.

Answer 2

It seems to me that it would be better for you to encapsulate the color names in the stream so that it can be easily expanded, and not add new branches:

 return Stream.of("red", "blue", "green") .filter(colourName::startsWith).findAny().orElse(""); 

Note that this solution depends on Java 8 threads and method references.

Answer 3

Personally, I think you can simplify it with something like

 if (str != null) { if (str.equals("red")) return "red"; else if (str.equals("blue")) return "blue"; } return ""; 

or a little more complicated switch

 if (str != null) { switch (str) { case "red": return "red"; case "blue"; return "blue"; } } return ""; 

Testing length and then executing substring() seems to be very expensive for me to String .

Edit

If you need to check only the leading characters, I would prefer

 if (str != null) { if (str.startsWith("red")) return "red"; else if (str.startsWith("blue")) return "blue"; } return ""; 

Answer 4

regular expression solution. ^ means start of line.

 public static String GetColor(String input) { String[] colors = {"red", "blue"}; for(String color : colors) { if(input.matches(String.format("^%s.*", color))) return color; } return ""; }