• taladar@sh.itjust.works
    link
    fedilink
    arrow-up
    5
    arrow-down
    3
    ·
    4 months ago

    There are a few very questionable things in there. Unwrap should literally never appear in production code so if your code base uses it so often that you want a short-hand syntax for it that calls into doubt everything else you wrote.

    • 5C5C5C@programming.dev
      link
      fedilink
      arrow-up
      3
      ·
      4 months ago

      I do think that specific point is catering too much to sloppy get-it-done-fast-and-don’t-think developers. It’s true that they are Rust’s most untapped demographic, and the language won’t reach the pinnacle of mainstream usage without getting buy-in from that group, but I really think they’ll be won over eventually by everything else the language and ecosystem offers, and .unwrap() won’t be such an unreasonable price for them to pay in the long run.

    • cbarrick@lemmy.world
      link
      fedilink
      English
      arrow-up
      5
      arrow-down
      2
      ·
      4 months ago

      Unwrap should literally never appear in production code

      Unwrap comes up all the time in the standard library.

      For example, if you know you’re popping from a non-empty vector, unwrap is totally the right too for the job. There are tons of circumstances where you know at higher levels that edge cases defended against at lower levels with Option cannot occur.

      • BB_C@programming.dev
        link
        fedilink
        arrow-up
        5
        ·
        edit-2
        4 months ago

        (DISCLAIMER: I haven’t read the post yet.)

        For example, if you know you’re popping from a non-empty vector, unwrap is totally the right too(l) for the job.

        That would/should be .expect(). You register your assumption once, at the source level, and at the panic level if the assumption ever gets broken. And it’s not necessarily a (local) logical error that may cause this. It could be a logical error somewhere else, or a broken running environment where sound logic is broken by hardware or external system issues.

        If you would be writing comments around your .unwrap()s anyway (which you should be), then .expect() is a strictly superior choice.

        One could say .unwrap() was a mistake. It’s not even that short of a shortcut (typing wise). And the maximumly lazy could have always written .expect("") instead anyway.

        • cbarrick@lemmy.world
          link
          fedilink
          English
          arrow-up
          1
          arrow-down
          2
          ·
          4 months ago

          Fair. But unwrap versus expect isn’t really the point. Sure one has a better error message printed to your backtrace. But IMO that’s not what I’m looking for when I’m looking at a backtrace. I don’t mind plain unwraps or assertions without messages.

          From my experience, when people say “don’t unwrap in production code” they really mean “don’t call panic! in production code.” And that’s a bad take.

          Annotating unreachable branches with a panic is the right thing to do; mucking up your interfaces to propagate errors that can’t actually happen is the wrong thing to do.

          • BB_C@programming.dev
            link
            fedilink
            arrow-up
            3
            ·
            4 months ago

            that’s not what I’m looking for when I’m looking at a backtrace. I don’t mind plain unwraps or assertions without messages.

            You’re assuming the PoV of a developer in an at least partially controlled environment.

            Don’t underestimate the power of (preferably specific/unique) text. Text a user (who is more likely to be experiencing a partially broken environment) can put in a search engine after copying it or memorizing it. The backtrace itself at this point is maybe gone because the user didn’t care, or couldn’t copy it anyway.

          • BB_C@programming.dev
            link
            fedilink
            arrow-up
            2
            arrow-down
            2
            ·
            4 months ago

            From my experience, when people say “don’t unwrap in production code” they really mean “don’t call panic! in production code.” And that’s a bad take.

            What should be a non-absolutest mantra can be bad if applied absolutely. Yes.

            Annotating unreachable branches with a panic is the right thing to do; mucking up your interfaces to propagate errors that can’t actually happen is the wrong thing to do.

            What should be a non-absolutest mantra can be bad if applied absolutely.

            • cbarrick@lemmy.world
              link
              fedilink
              English
              arrow-up
              2
              arrow-down
              1
              ·
              4 months ago

              You talk about “non-absolutist,” but this thread got started because the parent comment said “literally never.”

              I am literally making the point that the absolutist take is bad, and that there are good reasons to call unwrap in prod code.

              smdh

              • BB_C@programming.dev
                link
                fedilink
                arrow-up
                1
                arrow-down
                2
                ·
                4 months ago

                Don’t get angry with me my friend. We are more in agreement than not re panics (not .unwrap(), another comment coming).

                Maybe I’m wrong, but I understood ‘literally’ in ‘literally never’ in the way young people use it, which doesn’t really mean ‘literally’, and is just used to convey exaggeration.

                • taladar@sh.itjust.works
                  link
                  fedilink
                  arrow-up
                  2
                  ·
                  4 months ago

                  No, I actually meant it as in the traditional meaning of literally. As in

                  [lints.clippy]
                  unwrap_used = "warn"
                  expect_used = "warn"
                  

                  along with a pre-commit hook that does

                  cargo clippy -D warnings

                  (deny warnings).

                  There are always better ways to write an unwrap, usually via pattern matching and handling the error cases properly, at the very least logging them.

      • taladar@sh.itjust.works
        link
        fedilink
        arrow-up
        1
        ·
        4 months ago

        As a sysadmin I have sadly encountered too many situations where a programmer thought “oh, this will never happen” to agree with you that a code construct that provides no information to the user should be used in such cases.