Comparing Color objects with == This example comes from a code snippet posted on Stack Overflow:

private double brickVal(Color c) {
    if (c == Color.RED) {
        return 10.0;
    } else if (c == Color.ORANGE) {
        return 8.0;
    } else if (c == Color.YELLOW) {
        return 6.0;
    } else if (c == Color.GREEN) {
        return 4.0;
    } else if (c == Color.CYAN) {
        return 2.0;
    } else if (c == Color.MAGENTA) {
        return 1.0;
    } else {
        return 1.0;
    }
}


Is the use of == here correct / best practice?
No, it's not correct (and thus of course also not best practice). For example the condition c == Color.GREEN will be true if the method is called as brickVal(Color.GREEN), but false if it is called as brickVal(new Color(0, 255, 0)). Since Color.GREEN is equal to new Color(0, 255, 0) this behavior is most likely unintentional (at least I can't imagine a scenario where you'd want for brickVal(Color.GREEN) to behave differently than brickVal(new Color(0,255,0)).

Of course if you know that the method will only ever be called using the "pre-made" colors and never using new Color, it will behave correctly. However I'd still advise against using ==. I can see no good reason to not use equals and using == comes with the danger that someone might call brickVal with new Color anyway, not knowing that they're not supposed to.

Further given the fact that brickVal apparently is meant to be only called with some specific colors as arguments and it doesn't use any properties of the colors other than their identity, I think an enum might be more suitable here than using the Color class. This way you can use switch instead of if ... else if ... and don't have to worry about anybody passing in new Color as the argument.



As a somewhat unrelated note, I find it confusing that the brickVal variable is set in every case except if the argument is RED and in the else case. Obviously I don't know how and where the brickVal variable is going to be used, but this seems like a design smell to me. I also think it's a bad idea to have a variable with the same name as the method.