cleaner code suggestion to string manipulation

Question

So I put a lot of thought into the logic of my if statement in line 6 of my code. But I was hoping to get some feedback from more experienced developers.. Do you guys think I just over complicated my code and if so, how would you have written this more concise?

Answer 1

Over-complicating? Yes, you really are :-)

I would just be using the much simpler:

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

The following complete program shows it 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 that program being, as expected:

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

---

If you want it easily expandable in future, you could opt for 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 a color to the allow array is the only step you need to take to have it recognised.

Answer 2

It seems to me that you'd be better off encapsulating your colour names in a stream so that it can be easily expanded rather than having to add new if branches:

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

Note that this solution relies on Java 8 streams and method references.