• RagingRobot@lemmy.world
    link
    fedilink
    arrow-up
    5
    ·
    edit-2
    11 months ago

    This is why I usually don’t comment on stuff like this in PRs. If it’s readable and easy to understand it doesn’t need more abstractions. Even if it’s less code. What’s it save like a few bytes? That’s not as useful as the whole team instantly knowing how the code works when they see it lol

    I will say though if a jr dev came upon the last code they would just look it up and learn something so that’s a total valid path too. Just depends on your codebase and how your team works. I think it usually ends up being a mix with larger teams.

    • KairuByte@lemmy.dbzer0.com
      link
      fedilink
      arrow-up
      7
      ·
      11 months ago

      There’s more to it imho. The first three are more prone to mistakes than the last. You are much less likely to accidentally alter the logic intended in a simple null coalesce than you are in if statements.

      • RagingRobot@lemmy.world
        link
        fedilink
        arrow-up
        1
        arrow-down
        1
        ·
        11 months ago

        That’s fair but if you had proper test coverage there wouldn’t be much risk. Who has that though? Lol

        • KairuByte@lemmy.dbzer0.com
          link
          fedilink
          arrow-up
          4
          ·
          11 months ago

          None of my projects had time for reliable testing unfortunately. It was always “next sprint” or “when we have time” which never really came to fruition.

    • jhulten@infosec.pub
      link
      fedilink
      arrow-up
      1
      arrow-down
      1
      ·
      11 months ago

      Yeah, I think there is a tipping point between terse and magic. I might grimace a little at the first one, have no comment on the middle two, and definitely comment on the last one. Wrote code like the person troubleshooting it is on-call, mildly hung over, and it’s 3am.