2

After reading To ternary or not to ternary? and Is this a reasonable use of the ternary operator?, I gathered that simple uses of the ternary operator are generally accepted, because they do not hurt readability. I also gathered that having one side of the ternary block return null when you don't want it to do something is a complete waste.. However, I ran across this case while refactoring my site that made me wrinkle my nose:

if ($success) {
    $database->commit();
} else {
    $database->rollback();
}

I refactored this down to

$success ? $database->commit() : $database->rollback();

And I was pretty satisfied with it.. but something inside me made me come here for input. Exception catching aside, would you consider this an okay use case? Am I wondering if this is an okay use because I have never done this before, or because it really is bad practice? This doesn't seem difficult to me, but would this seem difficult to understand for anyone else? Does it depend on the language.. as in, would this be more/less wrong in C, C++, or Java?

Community
  • 1
  • 1
abelito
  • 1,094
  • 1
  • 7
  • 18
  • Consider that the ternary expression requires the two functions to have return values, even though the calling code ignores them. If the functions were later modified to not return a value, say because the return value is ignored everywhere, then your ternary expression would need to be modified -- to adapt to the absence of return values it was already ignoring. The 'if' statement is more resilient to change. – Andy Thomas Nov 29 '11 at 15:22

3 Answers3

2

No, it is not OK. You are turning something that should look like a statement into something that looks like an expression. In fact, if commit() and rollback() return void, this will not compile in Java at least (not sure about the others mentioned).

If you want a one-liner, you should rather create another method on the $database object such as $database->endTransaction($success) that does the if statement internally.

Ramon
  • 8,202
  • 4
  • 33
  • 41
  • I'm pretty sure it will not compile in Java if the `commit()` or `rollback()` method return types are `void` or do not have a common supertype. As for why it is wrong to make statements look like expressions, that is more subtle. Expression should generally not have side-effects - its just not what people expect and it makes the flow of control unclear. – Ramon Nov 24 '11 at 15:22
  • I like this answer a lot, as it removes the duplicate ternary code from my specific service calls and pushes it into one centralized location. I am definitely implementing this, thank you :) I misunderstood the first thing you said, but I see now what you meant. And I didn't realize that about Java. – abelito Nov 24 '11 at 15:26
  • Thought about it, and I still would probably not do the if statement interally, but I would continue using the ternary operator in that function as well like so: return $success ? $this->commit() : $this->rollback(); It is all about embracing brevity while protecting clarity. – abelito Nov 24 '11 at 15:40
  • 1
    "Expression should generally not have side-effects - its just not what people expect" - that's not at all language-agnostic. A possible bellwether: if your language has `++` and `--` operators, then expressions are expected to have side effects, or at least they are by people who have even vaguely realistic expectations. If the language doesn't have those operators or equivalents, then by omitting them it's making some kind of stand against side-effects in expressions, so perhaps it's worth programming with that constraint. – Steve Jessop Nov 24 '11 at 16:04
  • Steve, that is an awesome way to think about.. It never occurred to me to think "why can't I do this in this language?"... probably because I was too focused on trying to get something done, rather than examining why exactly the language I am using operates in this way. We must determine then if (in this abstract scenario) that feature is intentionally missing, or if it was unimplemented. – abelito Nov 25 '11 at 16:16
  • 1
    @abelito: For example, Python doesn't have `++` and `--`. There's no strict rule in Python that expressions can't have side-effects, they can and sometimes do. But as a point of style, Python libraries tend to define that mutating functions return `None`, just to nudge users to write anything with side-effects as a standalone statement. The classic example is that `mylist.sort()` modifies the list, and returns nothing so it's unlikely to be used as part of a larger statement. `sorted(mylist)` returns something: a *copy* of the list, the original is unmodified. It's a useful style. – Steve Jessop Nov 26 '11 at 17:56
1

I would be more inclined to use it in case the two actions are mutually-exclusive and/or opposite (yet related to each other), for example:

$success ? go_up() : go_down();

For two unrelated actions I would be less inclined to use it, the reason being that there is a higher probability for one of the branches to need expanding in the future. If that's the case, you will again need to rewrite it as an if-else statement. Imagine that you have:

$success ? do_abc() : do_xyz();

If at some point you decide that the first branch needs to do_def() as well, you'll need to rewrite the whole thing to an if-else statement again.

The more frequent usage of the ternary operator, however, is:

$var = $success ? UP : DOWN;

This way you are evaluating it as an expression, not as a statement.

Blagovest Buyukliev
  • 42,498
  • 14
  • 94
  • 130
  • Yeah, there is definitely a small niche for this type of usage, and I completely agree about the eventual need for expansion. I just thought of the case where the commit itself fails for whatever reason, and I need to capture that and display it on the screen for the user.. Well, all of a sudden I need to keep track of THAT success as well and return it. – abelito Nov 24 '11 at 15:33
  • I like your guidelines for using this. Ramon gave a really good alternative to what I was doing by suggesting to push this logic out of the service call and into the service itself for re-use purposes. However, I think the first half of your answer explained some solid guidelines for the use of the ternary operator in this way. I am still not convinced that there isn't a place in languages for this kind of use. – abelito Nov 24 '11 at 15:48
0

The real question is, "Is the ternary form more or less readable than the if form?". I'd say it isn't. But this is a question of style, not of function.

Ross Patterson
  • 9,527
  • 33
  • 48