This post was brought to you by this PMD rule.

Transcription

Why do we have this stupid code analyzer rule enabled anyway? Nobody writes code like this…

After telling them the lore why it’s there:

You have seen such things before?

11 Times, as a matter of fact

    • i_stole_ur_taco@lemmy.ca
      link
      fedilink
      arrow-up
      29
      arrow-down
      7
      ·
      4 days ago

      I think this is just a picky optimization.

      The first one runs the constructor to instantiate a new string, then gets its class (which is presumably a static property anyway). The second doesn’t have to run any constructor and just grabs the static class name from the type.

      Maybe there’s more implementation nuance here but it seems like an opinionated rule that has zero effect on performance unless that code is being called thousands of times every second. And even then the compiler probably optimizes them to the same code anyway.

      • spooky2092@lemmy.blahaj.zone
        link
        fedilink
        English
        arrow-up
        33
        ·
        4 days ago

        Maybe there’s more implementation nuance here but it seems like an opinionated rule that has zero effect on performance unless that code is being called thousands of times every second

        It’s good practice to get in the habit of coding to only do the things you want/need to do rather than hoping the compiler does it for you.

        This particular constructor call may be light, but there may be constructors that have a lot of overhead. Or you might be running alongside 1000 other processes who said the same thing and you start to see performance degradation.

        • Nat (she/they)@lemmy.blahaj.zone
          link
          fedilink
          arrow-up
          2
          ·
          4 days ago

          These things add up if you’re doing them all over a 1 million line codebase, by which point it’s incredibly painful to claw back performance if you need it.

          • ulterno@programming.dev
            link
            fedilink
            English
            arrow-up
            3
            ·
            edit-2
            3 days ago

            This seems like one of those cases where you don’t want to be waiting until benchmarking.

            It makes the code simpler anyway.

      • ryannathans@aussie.zone
        link
        fedilink
        arrow-up
        37
        ·
        4 days ago

        It’s not picky, needlessly creating objects makes the garbage collector run a lot more. Especially if it’s invoked frequently like Minecraft recreating the object for every block on the screen for every frame to render. The garbage collector is largely responsible for lag of up to a second occurring at random times.

      • Michal@programming.dev
        link
        fedilink
        arrow-up
        10
        arrow-down
        1
        ·
        3 days ago

        It’s like saying list.isEmpty() over list.getLength() == 0 is a picky optimisation.

        There’s a developer out there who coded this and they obviously don’t know what they’re doing. One day they’re gonna iterate all rows in the database to check if it’s empty. You have to flag these issues early and teach the newbies.

      • Ephera@lemmy.ml
        link
        fedilink
        English
        arrow-up
        19
        ·
        4 days ago

        Well, it also avoids running instantiation code, which could be doing all kinds of things. In theory, it could have a side-effect which modifies some of your application state or issues a log statement or whatever.

        Even if it doesn’t do anything wild right now, someone could change that in the future, so avoiding running such code when it’s not needed is generally a good idea.

    • Naich
      link
      fedilink
      arrow-up
      5
      ·
      4 days ago

      The first one returns an instance of the class, the second one is a static reference? I don’t really know what I’m on about, so the words might be wrong.