Writing code for both computers and humans | Hacker News Reader

Writing code for both computers and humans

tonymottaz.com

49 pointspfrrp29 comments

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/Refe...

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.

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.

Seems like the author fell into this trap

Yes, even better "isNumber()" because a function that tests a negative can cause confusion and mistakes.

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.

`isNaN` does return a boolean though, it doesn't return a normalized value.

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 :)

> 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-...>.

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?

This code doesn't. It guarantees that within `useControlledState`[1], `defaultValue` is either NaN, an actual number, a number-like string, or something else that can be coerced into one[2]. If `defaultValue` is, for example, `true` or even an empty array, then the result of the expression `isNaN(defaultValue) ? NaN : defaultValue` will pass through `true` or the empty array.

1. <https://github.com/adobe/react-spectrum/blob/main/packages/%...>

2. whether one of those non-numbers is prevented from actually making an appearance at this point for other _reasons_ is a different question, but the fact remains that one making its way through is not something that would be stopped from proceeding further by the isNaN check here

(Addendum to, I hope, clarify: I'll reiterate that it's the type annotations on the `useControlledState` call site parameterized as `useControlledState<number>` (and the TypeScript team's decision to type `isNaN` to take only parameters with a number type) that "guarantees" `defaultValue` is a number. But that guarantee is a soft one, i.e. not a guarantee at all, precisely because of mobile code and separate compilation; I can contrive a project right now that uses the Adobe Spectrum library and pass whatever I want to it—it's not like the Adobe devs' compilers are going to be able to stop me.)

PS: Gilad Bracha makes a similar point in this interview about pluggable types:

Newspeak and Pluggable Types. Software Engineering Radio. IEEE Computer Society. <https://www.se-radio.net/2009/07/episode-140-newspeak-and-pl...>.

> because it will of course evaluate to NaN anyway when undergoing the number treatment.

The point of typing is to make sure this situation doesn't happen.

I write a method to accept Number inputs. I don't want to do the work (whether it's thinking or writing tests) to make it a pleasant experience for someone to pass non-Number inputs to my function (intentionally by mistake).

Did you take note of the context in which this conversation is taking place? Because it doesn't make sense as a response within that context.

(My top-level reaction to TFA, for background. I left it out because no-one asked.)

> Writing code for both computers and humans

Show me what you got.

> isNaN(defaultValue) ? NaN : defaultValue

Yuck.

> This made me pause for a moment.

Of course it did. I still pause every time I re-read it!

> This ternary is much better. This is expert-level programming. This is how to write code with empathy for other programmers.

No, it makes me pause every time I read it, and I cannot follow the path from "made me pause" to "expert-level programming".

Back to your comment:

>> dynamically typed so[...]

> It's not about dynamic typing in this case.

It is about dynamic typing. isNaN should not exist (except for specific cases such as machine floats where NaN means something concrete).

> If you saw that, your spidey-sense might start tingling, “This is a number—did they consider the NaN case?”

I want tingle-free programming. Let the compiler do the tingling for me. A Number-which-may-or-may-not-be-a-Number is not statically typed. I don't care if it's in a .ts file.

There are 15 more references to NaN in that module and they do not make for easy reading:

    // Clamp to min and max, round to the nearest step, and round to specified number of digits
    let clampedValue: number;
    if (isNaN(step)) {
      clampedValue = clamp(parsed.current, minValue, maxValue);
    } else {
      clampedValue = snapValueToStep(parsed.current, minValue, maxValue, step);
    }

Why doesn't the above code match its comment? Why doesn't it look something like:

    // Clamp to min and max, round to the nearest step, and round to specified number of digits
    let output = input |> roundNearest(step)
                       |> clamp(min, max)
                       |> roundToDigits(numDigits)

Their code either calls into clamp() or snapValueToStep() based on whether 'step' is a number. Does that imply 'step' needs to be a number for the snapValueToStep case? If so, 'step' will need to be checked again inside that method. Why didn't they accept a typed number as input? Or - if they were forced to accept a string - check it at the top of the method? Oh wait - they did! Twice in two lines!

    let clampStep = !isNaN(step) ? step : 1;
    if (intlOptions.style === 'percent' && isNaN(step)) {
        clampStep = 0.01;
    }

So now 'step' and 'clampStep' are both in scope. They may or may not be numbers (insofar as the coder is willing to keep scrolling back up and re-reading code that at first "makes them pause", but then later makes them feel like it's "expert code".)

Why not:

    const clampStep = 
        case parseNum strStep of
            Just step -> step
            Nothing
                | intlOptions.style == Percent -> 0.01
                | otherwise                    -> 1

* One definition site, so you can read all possible values from the leaves {step, 0.01, 1}

* The possibly-a-String step has been demoted to 'strStep :: String', so that 300 lines later, the coder cannot not accidentally pass 'step' to another function (and if he did, the compiler would stop it immediately).

* clampStep is actually a number, so no more NaN checks.

But hey, that's all par for the course: Java programmers don't think they hit NPEs, C++ programmers don't think they have ("modern") memory issues, and dynamic programmers don't think they have type problems.

What really prompted me to comment was your suggestion that it would have been better to skip the confusing checks, keep passing through an untyped object, and let some downstream expression blow up.

Half of what you wrote here has nothing to do with the context of this discussion (again), and the half that's at least superficially relevant is materially irrelevant as a consequence of being a strawman.

> Java programmers don't think they hit NPEs, C++ programmers don't think they have ("modern") memory issues, and dynamic programmers don't think they have type problems

Aside from the obvious category error, you aren't talking to a "dynamic programmer" who "[doesn't] think they have type problems". (It may also shock you to learn that I'm not an advocate of NodeJS/NPM, the programming style exemplified by the Adobe project identified in the blog post, or even TypeScript. That doesn't change the fact that I'm right about the things I said before—and the fact that I am right means it wouldn't matter even if I were a "dynamic programmer", anyway.)

The _entire_ point of what I wrote is laid out in my first paragraph: "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." I even made it more explicit. This problem is not because of dynamic typing. This is a problem because of code mobility and separate compilation. I even provided references, FFS. You'd get the _same_ problem in languages that are not JS or TypeScript—including static languages. Why? Because this is a problem arising from code mobility and separate compilation—not static versus dynamic typing.

> What really prompted me to comment was your suggestion that it would have been better to skip the confusing checks

Not what I said, Holmes. The strength of your powers of divination don't merit the confidence you seem to have in them.

This is where I exit.

I don't know how you can think I'm arguing some mysterious, unrelated tangent.

--------------

>>>> {If `useControlledState` were just vanilla JS and no one were doing typechecking during development and build [1]}, there would have been {no problem with passing something else as an argument [2]}, because it will of course {evaluate to NaN anyway [3]} when undergoing the number treatment.

>>> The point of typing is to make sure this situation [3] doesn't happen.

>> Did you take note of the context in which this conversation is taking place? Because it doesn't make sense as a response within that context.

> What really prompted me to comment was your suggestion that it would have been better to {skip the confusing checks [1]}, {keep passing through an untyped object [2]}, and {let some downstream expression blow up [3]}.

--------------

>>> It's not about dynamic typing in this case.

>> It is about dynamic typing

I feel like you are so selectively blind to { isNaN(defaultValue) ? NaN : defaultValue; } being a wart of dynamic typing (not to mention TFA who had to slow down to understand it) that I couldn't do anything but submit other examples of selective blindness to see if they struck home:

>> Java programmers don't think they hit NPEs, C++ programmers don't think they have ("modern") memory issues, and dynamic programmers don't think they have type problems.

Why is it that you can write "People accustomed to strongly typed languages and thinking mostly of monolithic apps" but you bristle when I write "dynamic programmers"?

> The _entire_ point of what I wrote is laid out in my first paragraph: "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." I even made it more explicit. This problem is not because of dynamic typing.

It's crystal clear and I'm contradicting you, not missing your point.

What matters is not whether you—or any other confused commenter stuck in the second panel of the glowing brain meme—contradicts what I'm saying, but whether the facts do. They don't.

You have missed the point, and not just once. Not even twice.

> I feel like you are so selectively blind to { isNaN(defaultValue) ? NaN : defaultValue; } being a wart of dynamic typing

You seem selectively blind to the fact that (1) the code you are referring to is statically typed, not dynamic; and (2) that I have already addressed this—not even as part of some back-and-forth with you, but before you even waded in to this conversation with your condescending attempt to explain "the point of typing". (Gee, thanks.)

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.

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.

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.)

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.

I agree with the author that this is a reasonable way to indicate their intent. But I've seen so many accidentally ineffectual code snippets that if I saw this code I'd be inclined to delete it unless there was also a comment expressing its purpose.

And a comment on its own would be enough anyway

Yeah, ideally you’d have some kind of static typing to restrict the code to only use Number, and then a comment that says what the function does in case of NaN.

Yeah!

There was a thread the other day about a linter that flagged useless code and all the odd bugs it caught, and I think it would have flagged this snippet, eh?

"Interesting bugs caught by no-constant-binary-expression"

https://news.ycombinator.com/item?id=38196644

https://eslint.org/blog/2022/07/interesting-bugs-caught-by-n...

I don't know if that was the real intent of this code, considering all the weird nuance around NaN in Javascript - however I agree with the point the author makes, and will take it one step further:

We should be building these semantics directly into our languages, not relying on programmers to strictly follow a "best practice". In this case, it would be making values non-nullable and baking in Result/Option/etc style types that force programmers into handling the null (or NaN) case.

This communicates an important idea, Well-written code is not only correct and efficient, but it is also readable, maintainable, and understandable to other programmers.

This culture should be encouraged more to make other developer's life easier, The thing I do nowadays is that I would often switch perspectives now and then, When switching perspectives, if i become confused, i would work on making the code more meaningful.

Even if we are doing a solo project, if we come back to the code after a long while, the code should be greeting us with wide open hands rather than looking like an unexplored jungle.

When I'm programming I try to keep in mind what my future self would need. I take notes when I'm programming, and put those notes into the repo if it's a personal project, or in a separate repo just for the project. Mostly those notes are of little value to anyone by myself, but having them around lets me condense my thoughts down to comments or documents useful for other team members. With a little work, I can turn my notes into a variety of other kinds of useful documentation: references, explanations, FAQs, runbooks, etc.

Reply on news.ycombinator.com