4

I am using TSLint for static analysis of my TypeScript code.

One of the default rules does not allow the use of != to check for "not available" values. To explain what I mean by "not available", I will show an example:

/* This is supposed to return an object with a property token.
 * I need to use auth.token in my code but I have to be sure that I have a
 * value for the token. I do not control the code in that method. */
const auth = someService.getAuthentication();

Of course, I can try to guess, doing some tests, what the method returns when it does not return a token, but I do not like to write code based on something that seems to return a null or seems to return an undefined value.

I want to be sure that my code works in both cases.

Wouldn't it be better, in this case, to write:

if (auth.token != null) {...

instead of:

if (auth.token !== null && auth.token !== undefined) {...

?

I understand that a person that does not know JavaScript could miss the fact that != is changing the type of the operands, but someone who ignores that can probably still guess the meaning of that code.

Boann
  • 48,794
  • 16
  • 117
  • 146
Marco Altieri
  • 3,726
  • 2
  • 33
  • 47
  • 1
    Look here for another discussion on the topic: https://stackoverflow.com/questions/359494/which-equals-operator-vs-should-be-used-in-javascript-comparisons – Eriks Klotins Sep 23 '18 at 10:48
  • 1
    You could go either way. With the former, you probably need to do an in-line directive to disable that rule for just that line, otherwise the linter will still report it as a problem and you know it's not. The latter doesn't require a special exception but it's more code. All in all, it's the same thing and on average, you type the same. So just pick one and go with it. I'd personally prefer the latter approach because I don't like littering my code with linter directives. – VLAZ Sep 23 '18 at 10:49
  • 4
    Ideally, it should be documented – having two values represent the same meaning is an unnecessary source of complexity in many cases. (Just make sure it breaks on the other one instead of doing the wrong thing. And yes, there are important situations where `null` and `undefined` behave differently, like with ES6 default values.) But yes, if you want to allow both, `!= null` is a standard enough idiom for `!== null && !== undefined`, so you can configure your linter to allow it specifically. – Ry- Sep 23 '18 at 10:49
  • 2
    For null checks, what you are doing is very reasonable. `null` is only loosely equal to `null` or `undefined`. – Luca Kiebel Sep 23 '18 at 10:49
  • 1
    See https://stackoverflow.com/q/5101948/1048572 and https://stackoverflow.com/q/2647867/1048572 – Bergi Sep 23 '18 at 11:03
  • 3
    If you disagree with your linter rule, you should not enable that rule. (And yes, it is totally valid to disagree here) – Bergi Sep 23 '18 at 11:04
  • 1
    i believe this is because of tslint rules in your project: no-null-keyword: true. They have mentioned about it here... https://palantir.github.io/tslint/rules/no-null-keyword/ – Shadab Faiz Sep 23 '18 at 11:08

1 Answers1

1

In most cases, it's enough to :

if (auth && auth.token) {
    //.........
}
shawn
  • 4,305
  • 1
  • 17
  • 25
  • 1
    That's still not semantically the same as `auth.token != null`. – VLAZ Sep 23 '18 at 13:32
  • 1
    Whatever language we use, we should focus on the goal. The only question is: what is the purpose of doing XXX? IMO, in your API case, the only thing you care about is to get a valid token, I don't think we should spend much time on detecting the "type" (say, an empty string is still an invalid token). – shawn Sep 24 '18 at 09:31
  • The question isn't "what is a valid token" but "how do I handle `!=null`" Hence, I believe the answer should focus on the latter, not the *example* given. The same code base could easily have another function called `findName` or `findAge` that suffers from the same `!= null` problem. In that case your solution could fail because it doesn't actually stick to the goal stated. Again, that goal is "what do I do if code might return either `undefined` or `null` and how do I check for those two - either explicitly through `===` twice or implicitly through `!= null` once". – VLAZ Sep 24 '18 at 09:40
  • It is still nice to use `if (result.name)` or `if (result.age)` when doing `findName` or `findAge` . Isn't this style shorter and more beautiful, and still correct? In most cases when you want to check undefined or null you do not want other falsy values either. If you want to distinguish the undefined or null and other falsy values, `if(v != null)` is fine IMO because: (1) easier to read. (2) same as other languages (3) no worry about others do not understand javascript because the result is just correct. – shawn Sep 25 '18 at 03:53
  • `Isn't this style shorter and more beautiful, and still correct?` not if `findAge` returns `0` when looking up a baby. Or when `findName` returns `""` when something doesn't have a name yet which is a valid state to be in. There are plenty of time when zero or an empty string are valid outputs and plenty of times people sweep them under the rug because they just disregard any falsey values which comes back to bite them. – VLAZ Sep 25 '18 at 04:02
  • I said `in most cases`. @vlaz – shawn Sep 25 '18 at 04:04
  • If something returns a string or a number in most cases it doesn't automatically fall under "most cases". Would you also go with your suggested solution if you're doing `calculateVAT`? If it's fine "in most cases", surely you should again dismiss any falsey values from that. However, a zero is a perfectly valid output. `getShippingTax`? `applyDiscount`? Are they also going to be fine "in most cases"? What about `person.title`? Any time you are dealing with strings or numbers you risk discarding valid values that happen to be falsey. – VLAZ Sep 25 '18 at 04:12
  • Good example. `applyDiscount` is surely in the most cases. `if (o.discount) { amount = price * (1 - o.discount) }` . Everything correct, no worry about `0`, `null`, `undefined` – shawn Sep 25 '18 at 04:29
  • `person.title` can also be in the most cases. If a person doesn't have a title, what do you want to do? just hide the html element in UI, right? Everything correct again. – shawn Sep 25 '18 at 04:30