CODEWITHSUNDEEP
javaternary-operator
Carter Hudson
Carter Hudson

Reputation: 1263

Conditional method calling by abusing ternary wrapped in if statement?

Recently I was doing a code review and came across this guy:

if(!sharePermission.isExpired() ? activePermissions.add(sharePermission) : expiredPermissions.add(sharePermission));

Basically using a ternary expression to call methods that return a boolean value and wrapping it in an if(...) statement to satisfy the requirement of being a standalone statement. Is this more or less valid than 

if(!sharePermission.isExpired())
   activePermissions.add(sharePermission); 
else 
   expiredPermissions.add(sharePermission);

if you really need to condense your code to one line? Is any sort of extra space allocated for the value returned from the ternary expression when wrapped in the if(...) ?

I'm not a fan of either of them, just curious.

Upvotes: 1

Views: 377

Answers (3)

Naman
Naman

Reputation: 31878

The ternary equivalent for the if..else that you are looking for is something like - 

(!sharePermission.isExpired() ? activePermissions : expiredPermissions).add(sharePermission); // no if here

is equivalent to 

if(!sharePermission.isExpired()) {
    activePermissions.add(sharePermission); 
}
else {
    expiredPermissions.add(sharePermission);
}

Upvotes: 0

Andy Turner
Andy Turner

Reputation: 140319

It's an abuse of the if statement to do that, never mind the conditional expression.

It would be cleaner to write either a full if statement, or to use the conditional operator to select a list to add to:

List<Permission> list = isExpired() ? expiredPermission : activePermission;
list.add(sharePermission);

Upvotes: 5

Sergey Kalinichenko
Sergey Kalinichenko

Reputation: 726509

There is no assignment happening, only an evaluation of a boolean condition. No extra memory is allocated for the result of the evaluation.

Using a ternary expression to emulate a ternary statement, however, is grossly unorthodox. It is going to reduce readability of your code, without bringing any additional benefit. Hence, using a plain if with an else is a better alternative.

Note that if activePermissions and expiredPermissions are of the same type, you can use a ternary expression to decide between the targets of the add call, as follows:

(sharePermission.isExpired() ? expiredPermissions : activePermissions).add(sharePermission);

Upvotes: 3

Related Questions