Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Conditional operator in if-statement?

I've written the following if-statement in Java:

if(methodName.equals("set" + this.name) ||
    isBoolean() ? methodName.equals("is" + this.name) :
                  methodName.equals("get" + this.name)) {
    ...
}

Is this a good practice to write such expressions in if, to separate state from condition? And can this expression be simplified?

like image 387
Pindatjuh Avatar asked May 12 '10 23:05

Pindatjuh


4 Answers

I would change it to

if (methodName.equals("set" + this.name)
 || methodName.equals( (isBoolean() ? "is" : "get") + this.name)) {
    ...
}
like image 155
SLaks Avatar answered Sep 22 '22 09:09

SLaks


Is it good practice? It's good if it makes it easier to read. It makes it easier to read if (1) it does and (2) the sort of person who'd be confused by it won't be reading it. Who's going to read it?

like image 24
amara Avatar answered Sep 18 '22 09:09

amara


Wouldn't something like the following work?

if (methodName.equals("set" + this.name)
    || methodName.equals("get" + this.name)
    || (isBoolean() && methodName.equals("is" + this.name))) {
    ...
}

It's more readable than the way in which you used the ternary operator and certainly easier to understand. It also has the advantage that it can avoid an unnecessary method call to the isBoolean method (it has either 1, 2 or 4 method calls whereas yours always has either 1 or 3; the performance gain/loss is probably too minute to notice).

Also there's a similar question here titled "Is this a reasonable use of the ternary operator?" One user had the following to say:

The ternary operator is meant to return a value.

IMO, it should not mutate state, and the return value should be used.

In the other case, use if statements. If statements are meant to execute code blocs.

Do note that I included parentheses around the expression containing '&&' for readability. They aren't necessary because x && y is evaluated before m || n.

Whether you choose to use it is up to you, but I tend to avoid it in favour of readability.

like image 36
Dustin Avatar answered Sep 22 '22 09:09

Dustin


I would be inclined to change it to

if (methodName.equals(setterForThis())
   || methodName.equals(getterForThis())) {
    ...
}

with some functions extracted:

private String setterForThis() {
   return "set" + this.name;
}

private String getterForThis() {
   return (isBoolean() ? "is" : "get") + this.name;
}

It's longer of course, but I'm not really into golf anyway.

like image 32
Don Roby Avatar answered Sep 21 '22 09:09

Don Roby