9 ms·

I don't think what the author says is the intent of the code. isNaN() returns true not just for NaN but for anything that is "not a number"[1]. For example, it

I don't think what the author says is the intent of the code. isNaN() returns true not just for NaN but for anything that is "not a number"[1]. For example, it returns true for things like "hello". So it just canonicalizes everything that is not a number into NaN. [1] https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/isNaN https://developer.mozilla.org/en-US/docs/Web/JavaScript/Refe...

abecedarius 3y ago
There's a whole set of different floating-point values which are NaN, and this canonicalizes them into one particular NaN, as I understand it. I'm not even sure that covering type conversions was intended here.
anonzzzies 3y ago
This is my issue with js; isNaN looks related to NaN in a 1-1 mapping as in, testing ‘x is NaN’ but it doesn’t. So the naming is confusing as it does more than that, isNotNumeric() or so.
- jalk 3y ago
  Seems like the author fell into this trap
- eloisant 3y ago
  Yes, even better "isNumber()" because a function that tests a negative can cause confusion and mistakes.
- tetha 3y ago
  I am ready to oppose any `isFoo` function that doesn't return a boolean. If you ask "Is this a Nan", why would you expect 42 as an answer? Or, "hello". Kinda feels like "Are you at home?" - "Kitchen". It eventually becomes a "yes" through more thought, but eh... I'd much rather call this "normalizeNaN" or something.
  - Jtsummers 3y ago
    `isNaN` does return a boolean though, it doesn't return a normalized value.
stinos 3y ago
Funny. If this is effectively so, the code is actually fairly ok and clearly expresses the intent for humans yet the OP claiming exactly that managed to still misunderstand the code because they made up a precondition ('defaultValue is going to be a number') probably exactly because they refused to 'To answer this question, you need to start looking around for clues' so they missed the clue [1]; or maybe rather the clue is: dynamically typed so expecting a number is a wrong assumption. Still I'd argue a comment explaining why it's in this particular case fine that any non-sane input translates to NaN and not an error might be worth it Well, unless that is clear from the surrounding code :)
- cxr 3y ago
  > dynamically typed so[...] People accustomed to strongly typed languages and thinking mostly of monolithic apps always pin things on dynamic typing even where it's not the cause. It's not about dynamic typing in this case. What's going on here is partially because of separate compilation and partially because of mobile code (as Lars Bak explains[1]). Even if developing in a strongly typed language, the machinery would still have to take care to deal with the same thing. In fact, this isn't even JS; this file is TypeScript—and this expression being strongly typed is almost certainly exactly why this code was written this way: because `useControlledState<number>` demands it. If `useControlledState` were just vanilla JS and no one were doing typechecking during development and build, there would have been no problem with passing something else as an argument, because it will of course evaluate to NaN anyway when undergoing the number treatment. (This is kind of subtle, but it's not that subtle. Too many people make this mistake, and there's probably something that needs to be done about that; "because dynamic languages" has become something of a thought-terminating cliché that leads people to the wrong conclusions—as demonstrated even in the interview, at the point where Bak felt prompted to point this out.) 1. Inside V8 — A Javascript Virtual Machine. Going Deep: Expert to Expert. Accessed July 3, 2020. <https://channel9.msdn.com/Shows/Going+Deep/Expert-to-Expert-Erik-Meijer-and-Lars-Bak-Inside-V8-A-Javascript-Virtual-Machine https://channel9.msdn.com/Shows/Going+Deep/Expert-to-Expert-...>.
  - stinos 3y ago
    Funny. Seems I fell into the same trap I just sketched, making wrong assumptions because of not looking around in the code. But to be honest from your explanation I still cannot tell whether I merely used incorrect terminology wrt dynamic typing (but the general principle still holding) or whether this code guarantees that defaultValue is in fact always a numeric type?
laurentlb 3y ago
Thanks, your explanation makes a lot more sense. A short comment would be useful in the original code. Written as is, the original code looks surprising. I've seen this kind of easily simplifiable multiple times in large codebases, after years of refactoring and automated transformations.
lo_zamoyski 3y ago
Yeah, indeed, and this makes a whole lot more sense. I can sympathize with the argument of the OP, but I wouldn't exaggerate its profundity either. The intent was opaque enough for the OP that he got it completely wrong, and even under the OP's assumptions, the intent was still more opaque than necessary. Furthermore, you're still passing around `NaN`, which is sort of an analogue of passing around `null`. This example suggests that perhaps a better language construct ought to be used, like the `Option` type.
layer8 3y ago
Even if the author was right about the intent, I would disagree with them that “this is how to write code with empathy for other programmers”, because the code would just make me think “I must be missing something” (and indeed the author did miss something). My theory, by the way, was that the expression is normalizing the possible NaN representations to a definite one, possibly to prevent transporting information via the NaN representation. Such “what the …?” code should have a comment explaining the intent (e.g. “replace non-numeric values by NaN”). If it is used in multiple places, that’s a good opportunity to define a function for it, so the intent only needs to be documented in a single place. (In programming languages with annotations as a language construct, annotations could alternatively also be used for referencing the documentation of a coding pattern from multiple places.)
- josephg 3y ago
  Yeah I absolutely agree. The intent of that code isn’t clear - and if you need more proof, look how many different opinions there are in this thread about what that code is trying to do! Anyway, if they’re trying to coerce non-numbers into NaN, I’d write it like this: typeof defaultValue === “number” ? defaultValue : NaN I think that’s much more clear about the intent. I wouldn’t be confused by that code like I was confused by the code cited in the article.

abecedarius 3y ago

There's a whole set of different floating-point values which are NaN, and this canonicalizes them into one particular NaN, as I understand it. I'm not even sure that covering type conversions was intended here.

anonzzzies 3y ago

This is my issue with js; isNaN looks related to NaN in a 1-1 mapping as in, testing ‘x is NaN’ but it doesn’t. So the naming is confusing as it does more than that, isNotNumeric() or so.

jalk 3y ago
Seems like the author fell into this trap
eloisant 3y ago
Yes, even better "isNumber()" because a function that tests a negative can cause confusion and mistakes.
tetha 3y ago
I am ready to oppose any `isFoo` function that doesn't return a boolean. If you ask "Is this a Nan", why would you expect 42 as an answer? Or, "hello". Kinda feels like "Are you at home?" - "Kitchen". It eventually becomes a "yes" through more thought, but eh... I'd much rather call this "normalizeNaN" or something.
- Jtsummers 3y ago
  `isNaN` does return a boolean though, it doesn't return a normalized value.

stinos 3y ago

Funny. If this is effectively so, the code is actually fairly ok and clearly expresses the intent for humans yet the OP claiming exactly that managed to still misunderstand the code because they made up a precondition ('defaultValue is going to be a number') probably exactly because they refused to 'To answer this question, you need to start looking around for clues' so they missed the clue [1]; or maybe rather the clue is: dynamically typed so expecting a number is a wrong assumption. Still I'd argue a comment explaining why it's in this particular case fine that any non-sane input translates to NaN and not an error might be worth it Well, unless that is clear from the surrounding code :)

cxr 3y ago
> dynamically typed so[...] People accustomed to strongly typed languages and thinking mostly of monolithic apps always pin things on dynamic typing even where it's not the cause. It's not about dynamic typing in this case. What's going on here is partially because of separate compilation and partially because of mobile code (as Lars Bak explains[1]). Even if developing in a strongly typed language, the machinery would still have to take care to deal with the same thing. In fact, this isn't even JS; this file is TypeScript—and this expression being strongly typed is almost certainly exactly why this code was written this way: because `useControlledState<number>` demands it. If `useControlledState` were just vanilla JS and no one were doing typechecking during development and build, there would have been no problem with passing something else as an argument, because it will of course evaluate to NaN anyway when undergoing the number treatment. (This is kind of subtle, but it's not that subtle. Too many people make this mistake, and there's probably something that needs to be done about that; "because dynamic languages" has become something of a thought-terminating cliché that leads people to the wrong conclusions—as demonstrated even in the interview, at the point where Bak felt prompted to point this out.) 1. Inside V8 — A Javascript Virtual Machine. Going Deep: Expert to Expert. Accessed July 3, 2020. <https://channel9.msdn.com/Shows/Going+Deep/Expert-to-Expert-Erik-Meijer-and-Lars-Bak-Inside-V8-A-Javascript-Virtual-Machine https://channel9.msdn.com/Shows/Going+Deep/Expert-to-Expert-...>.
- stinos 3y ago
  Funny. Seems I fell into the same trap I just sketched, making wrong assumptions because of not looking around in the code. But to be honest from your explanation I still cannot tell whether I merely used incorrect terminology wrt dynamic typing (but the general principle still holding) or whether this code guarantees that defaultValue is in fact always a numeric type?

laurentlb 3y ago

Thanks, your explanation makes a lot more sense. A short comment would be useful in the original code. Written as is, the original code looks surprising. I've seen this kind of easily simplifiable multiple times in large codebases, after years of refactoring and automated transformations.

lo_zamoyski 3y ago

Yeah, indeed, and this makes a whole lot more sense. I can sympathize with the argument of the OP, but I wouldn't exaggerate its profundity either. The intent was opaque enough for the OP that he got it completely wrong, and even under the OP's assumptions, the intent was still more opaque than necessary. Furthermore, you're still passing around `NaN`, which is sort of an analogue of passing around `null`. This example suggests that perhaps a better language construct ought to be used, like the `Option` type.

layer8 3y ago

Even if the author was right about the intent, I would disagree with them that “this is how to write code with empathy for other programmers”, because the code would just make me think “I must be missing something” (and indeed the author did miss something). My theory, by the way, was that the expression is normalizing the possible NaN representations to a definite one, possibly to prevent transporting information via the NaN representation. Such “what the …?” code should have a comment explaining the intent (e.g. “replace non-numeric values by NaN”). If it is used in multiple places, that’s a good opportunity to define a function for it, so the intent only needs to be documented in a single place. (In programming languages with annotations as a language construct, annotations could alternatively also be used for referencing the documentation of a coding pattern from multiple places.)

josephg 3y ago
Yeah I absolutely agree. The intent of that code isn’t clear - and if you need more proof, look how many different opinions there are in this thread about what that code is trying to do! Anyway, if they’re trying to coerce non-numbers into NaN, I’d write it like this: typeof defaultValue === “number” ? defaultValue : NaN I think that’s much more clear about the intent. I wouldn’t be confused by that code like I was confused by the code cited in the article.