Hacker News new | ask | show | jobs
by jiaweihli 3436 days ago
Unfortunately, ternary operators eventually end up like this due to refactoring blindness:

  usefulMetric = wantComplexCalc ? 
                   (complexity > 40 ?
                     superComplexCalc(foo) :
                     regularComplexCalc(foo)) :
                   simpleCalc(foo);
3 comments

At some point you have to rely on policy and not language constraints. I submit that no language is constrained enough to protect against refactoring stupidity while also being flexible enough to be useful to the average programmer on the average project. If not ternary if, it will be something else. So, do you throw out every alternative method to accomplish the same thing, or do you put policies in place to keep the code sane, such as "no chained ternary operators are allowed" ?
In all honesty, I prefer having rules that have no special-case 'unless' issues. It's too much effort/trouble to remember all the cases where things don't work. I'm a good engineer but a terrible compiler.

I believe part of learning a new library/framework/language is to limit yourself to a certain subset of the API offered. After working with Ruby (the language) and Javascript (the ecosystem), I feel like that's the only way to preserve your sanity and productivity. I don't need to know 4 different ways of creating a lambda in Ruby, selecting 1 that can express the other 4 is good enough.

---

In this case, the rule would be no ternary operators, since they work well unless you nest them or unless you make them long/complicated.

Other examples -

You don't need to wrap if conditions unless you have a multi-line body:

  if (myCondition)
    x = 42;
    y = 23;
Early returns simplify short circuiting logic unless your function becomes too long:

  if (myVariableAtBeginningOfFunction) {
    return true;
  }
  ...
  // 2 screens later
  ...
  if (x == 42) {
    return false;  // why am I not getting false?!
  }
Using a variable as a conditional in javascript to test against undefined works well unless the value can be falsy:

  if (person.isStudent) {
    showSchool();
  }

  if (person.age) {
    showBirthCertificate(); // what if age is 0?
  }
> the rule would be no ternary operators

Why not "no nested ternary operators"?

> You don't need to wrap if conditions unless you have a multi-line body

Why not "keep unwrapped if conditions on a single line"?

> Early returns simplify short circuiting logic unless your function becomes too long

Why not "keep functions short"?

> Using a variable as a conditional in javascript to test against undefined works well unless the value can be falsy

Why not "only use conditionals on boolean values"?

I'm not saying your rules are right or wrong, I actually follow a couple of them myself, but your wording implies that other people are simply not following rules, or their rules have a lot of nuances and special cases, but the reality is more likely that their rules are different.

Ultimately we all make different connections and form different patterns in our head. As long as a team can agree on a code style, within a few months everyone starts developing the same cognitive patterns.

It's because it's too easy to lose some of these nuances during refactoring/development blindness. I'd go so far as to say it's inevitable.

If you come into a 3 year old codebase and during the first 2 weeks you need to add extra functionality to a 30-line function with an early return, are you going to refactor the early return? Or are you going to extend it into a 32-line function? What about the new hire after you?

Alternatively, your team has decided to embrace the "only use conditionals on boolean values" philosophy. You're working with a section of code that reads `if (myVar)`. It's been 3 hours, and you don't understand why the code's not working. Suddenly, you realize that at some point `myVar` was refactored from a non-nullable boolean to a nullable number, and someone missed changing this.

And the biggest offender yet - code that is grouped within a file into 'logical sections'. I've never seen this work out. What is a logical grouping for you is a confusing pairing for me. Or maybe it's that I can't immediately grok all 2000 lines of a file I've never seen before, and know where to place the method. This madness around code location is one of the quickest ways to code rot.

---

The perplexing thing to me is that these situations are completely preventable.

If you don't use early returns, scenario 1 won't happen.

Scenario 2 won't happen if you use real comparisons e.g. `if (person.age !== undefined)` (Similarly, `if (person.age != null)` breaks when null and undefined start meaning different things...)

And lastly, a canonical alphabetical/visibility ordering for methods in a file of any length is unambiguous. I don't care what the order is, as long as there is a canonical order.

---

I understand that other teams have their own rules. It's no trouble at all to adjust to things that are purely syntactic differences. But when the rules that are chosen hide lurking semantic pitfalls...I don't know why you'd risk shooting yourself in the foot.

A lot of my strong feelings on code style come from the book Code Complete. I highly recommend that to everyone who hasn't read it. It's filled with examples of confusing/broken code you might inherit, and teaches you how to avoid creating it yourself.

Edit: looks like we hit the HN thread depth limit. Happy to continue this over Twitter, check my profile.

That's just laziness on part of the refactorer. At that point, you need to use an outer if-else statement. Ternary operators are confusing when nested.
what about:

  usefulMetric = wantComplexCalc == false ? simpleCalc(foo)
               : complexity <= 40         ? regularComplexCalc(foo)
               : /* else */                 superComplexCacl(foo)
               ;
Doesn't look much better.