• FizzyOrange
    link
    fedilink
    arrow-up
    1
    ·
    20 hours ago

    There are arguments to be made either way, but normally you’d scope your variables in a way that the ones specific to a particular bit of code are not accessible from elsewhere.

    Sounds like you agree with that one to me? I’m not sure I follow their arguments about regions there (I’ve never used regions), but the example of declaring a variable in a block way before it is every used is spot on. I’ve seen code written like that and 99% of the time it’s a bad idea. I think a lot of it comes from people who learnt C where you have to do that (or maybe Javascript which has weird rules for var).

    Suggest writing a custom class to do what most languages can solve with inheritance or even better: the ? syntax.

    Yeah I’ll give you that one. They even suggest using Optional as a solution, which is what their “smelly” code did in the first place!

    Yes, it can be annoying. No, clarity is more important than insisting on removing that extra underscore.

    Not sure what your point is here. Of course inconsistent naming is a code smell. Do you want inconsistent names?

    They’re advocating the use of a function to replace an expression. Sometimes this works, but the task of a boolean expression is not always easily expressed in a couple words. And so you can end up with misleading function names. Instead, just put a comment in the code.

    Erm, yeah that’s why this is a code smell. They aren’t saying never have complex boolean expressions - just that if you do you’d better have a good reason because probably you’d be better off splitting it up into named parts.

    callback hell - Not even a code smell. It’s an issue from back when languages like JavaScript didn’t support promises yet, but callbacks were popular.

    Indeed, so now it is a code smell.

    • Boomkop3@reddthat.com
      link
      fedilink
      arrow-up
      1
      ·
      19 hours ago

      They aren’t saying never have complex boolean expressions…

      That’s not what I’m saying either. But I think this is to be judged on a case by case basis. And it can depend on your understanding of the context. I think there’s simply too much nuance here to just say “this smells”

      • FizzyOrange
        link
        fedilink
        arrow-up
        1
        ·
        14 hours ago

        I disagree. I’ve seen very complex boolean expressions and they were clearly code smell. Sometimes acceptable but definitely a fertile area for refactoring.

        Their example is crap to be fair - two comparisons is not complex.

        • Boomkop3@reddthat.com
          link
          fedilink
          arrow-up
          1
          ·
          12 hours ago

          If you do have a super complex boolean expression, I wouldn’t call it a smell. It’ll be much more obvious

    • Boomkop3@reddthat.com
      link
      fedilink
      arrow-up
      1
      ·
      19 hours ago

      Not sure what your point is here. Of course inconsistent naming is a code smell. Do you want inconsistent names?

      Of course not. But in some (uncommon in my experience) cases method names can be unclear or just plain impractically long. In such cases, I would rather see an exception to the rule than having to rely on a comment to explain the name choice.

      I had a great example a couple months back, but I can’t remember it right now. But here’s a (bad) example of such a situation.

      An example of this could be a button that triggers a click. You might call it BtnClick. Then the click event for it could be BtnClickClick. In this case, I’d rather see BtnClick_click. Ugly? Yes. Bad example? yes. But the idea is that it’s more clear that the _Click action is seperate from the name.

      • FizzyOrange
        link
        fedilink
        arrow-up
        1
        ·
        14 hours ago

        an exception to the rule

        There is no rule. Smells are not rules.

    • Boomkop3@reddthat.com
      link
      fedilink
      arrow-up
      1
      ·
      19 hours ago

      There are arguments to be made either way, but normally you’d scope your variables in a way that the ones specific to a particular bit of code are not accessible from elsewhere.

      They’re arguing to do this:

      int field = 1;
      void may() {
         do(field);
      }
      
      int field = 3;
      void you() {
         do(field);
      }
      
      int field = 3;
      void be() {
         do(field);
      }
      
      int field = 7;
      void happy() {
         do(field);
      }
      

      rather than

      int field = 1;
      int field = 3;
      int field = 3;
      int field = 7;
      
      void may() {
         do(field);
      }
      void you() {
         do(field);
      }
      void be() {
         do(field);
      }
      void happy() {
         do(field);
      }
      

      A bad example of encapsulation would be:

      class AClass {
          private class HelloThere {
               int a = 1;
               int b = 3;
               int c = 3;
               int d = 7;
               void DoStuff(AClass self) {
                    Do(a, b);
               }
          }
          private HelloThere field = new();
          void World() {
              field.DoStuff(this);
          }
      }
      

      Of course, there is nuance here. Is this class encapsulating enough that it’s got a right to exist? That’ll depend on the situation.

      Also, c has local static variables. Depending on your use case, it might just be easier in c than in C# and similar.

      // a method with a state, horrid in some contexts, great in others
      void PrintCounter() {
          static int count = 0;
          Print(count);
          count += 1;
      }
      

      And just in case you’re still reading and curious:

      #region PingPong
          // hi! I am in a region, collapse me using your ide!
      #endregion
      
        • Boomkop3@reddthat.com
          link
          fedilink
          arrow-up
          1
          ·
          edit-2
          12 hours ago

          In most cases it’s a bad idea, yes.

          Also, have another look at that example code snippet though: that static variable is local to that function. It’s a weird feature in c.

          I’ve used it quite often in embedded code where a single variable was only for one function, and only for that one app/device. Wrapping it in a struct would’ve made the code needlessly more complex (that’s a code smell). And yet, these static locals are very easy to refactor to one local to a struct. May the situation change, that’s still an option.

          • FizzyOrange
            link
            fedilink
            arrow-up
            1
            ·
            12 hours ago

            that static variable is local to that function

            Yes I know how static storage durations work. It’s still global state, which is a code smell. Actually I’d go as far as to say global state is just bad practice, not just a smell. Occasionally it’s the only option, and it’s definitely the lazy option which I won’t claim to never take!

            • Boomkop3@reddthat.com
              link
              fedilink
              arrow-up
              1
              ·
              11 hours ago

              Aaand… you didn’t even bother to google it :/

              This is not about storage durations, and it’s local to a function

              • FizzyOrange
                link
                fedilink
                arrow-up
                1
                ·
                1 hour ago

                I don’t need to Google anything. I have 30 years experience writing C & C++.

                This is not about storage durations

                Yes it is.

                https://en.cppreference.com/w/c/language/storage_duration

                it’s local to a function

                Only the visibility is local. The data is still global state. You can call that function from anywhere and it will use the same state. That’s what global state means.

                https://softwareengineering.stackexchange.com/a/314983

                Some of the biggest issues with global state are that is makes testing difficult and it makes concurrent code more error-prone. Both of those are still true for locally scoped static variables.