Skip to content
  • Categories
  • Recent
  • Tags
  • Popular
  • World
  • Users
  • Groups
Skins
  • Light
  • Cerulean
  • Cosmo
  • Flatly
  • Journal
  • Litera
  • Lumen
  • Lux
  • Materia
  • Minty
  • Morph
  • Pulse
  • Sandstone
  • Simplex
  • Sketchy
  • Spacelab
  • United
  • Yeti
  • Zephyr
  • Dark
  • Cyborg
  • Darkly
  • Quartz
  • Slate
  • Solar
  • Superhero
  • Vapor

  • Default (No Skin)
  • No Skin
Collapse
Code Project
  1. Home
  2. The Lounge
  3. Multiple returns from methods or clean code flow

Multiple returns from methods or clean code flow

Scheduled Pinned Locked Moved The Lounge
questiondiscussioncryptographycollaborationhelp
85 Posts 40 Posters 0 Views 1 Watching
  • Oldest to Newest
  • Newest to Oldest
  • Most Votes
Reply
  • Reply as topic
Log in to reply
This topic has been deleted. Only users with topic management privileges can see it.
  • F Offline
    F Offline
    Forogar
    wrote on last edited by
    #1

    I have just had a heated argument cage fight lively discussion with some of my team members about ReSharper's suggestion of refactoring code to replace nested ifs with a series of multiple early return statements. This caused horribly messy code that ReSharper actually described in it's help as "more readable"! Using a version of their example code (which is a lot simpler than the actual code in question):

    void SomeFunction(SomeThing s)
    {
    if (s != null)
    {
    if (s.Thing != null)
    {
    // Do Some Process with s.Thing
    .
    .
    .
    }
    }
    }

    becomes:

    void SomeFunction(SomeThing s)
    {
    if (s == null) return
    if (s.Thing == null) return;
    // Do Some Process with s.Thing
    .
    .
    .
    }

    This makes a complete hash of the natural flow of the code and introduces an execution statement (return) on the same line as the "if" which is bad coding practice, and then does it again, and then drops though to do some actual processing. If I was to refactor the code it would do this:

    void SomeFunction(SomeThing s)
    {
    if (s != null && s.Thing != null)
    {
    // Do Some Process with s.Thing
    .
    .
    .
    }
    }

    or possibly:

    void SomeFunction(SomeThing s)
    {
    if (s?.Thing != null)
    {
    // Do Some Process with s.Thing
    .
    .
    .
    }
    }

    Which is a lot cleaner! ...and this isn't even considering a method that returns a value of some sort. Opinions?

    - I would love to change the world, but they won’t give me the source code.

    P L S OriginalGriffO J 38 Replies Last reply
    0
    • F Forogar

      I have just had a heated argument cage fight lively discussion with some of my team members about ReSharper's suggestion of refactoring code to replace nested ifs with a series of multiple early return statements. This caused horribly messy code that ReSharper actually described in it's help as "more readable"! Using a version of their example code (which is a lot simpler than the actual code in question):

      void SomeFunction(SomeThing s)
      {
      if (s != null)
      {
      if (s.Thing != null)
      {
      // Do Some Process with s.Thing
      .
      .
      .
      }
      }
      }

      becomes:

      void SomeFunction(SomeThing s)
      {
      if (s == null) return
      if (s.Thing == null) return;
      // Do Some Process with s.Thing
      .
      .
      .
      }

      This makes a complete hash of the natural flow of the code and introduces an execution statement (return) on the same line as the "if" which is bad coding practice, and then does it again, and then drops though to do some actual processing. If I was to refactor the code it would do this:

      void SomeFunction(SomeThing s)
      {
      if (s != null && s.Thing != null)
      {
      // Do Some Process with s.Thing
      .
      .
      .
      }
      }

      or possibly:

      void SomeFunction(SomeThing s)
      {
      if (s?.Thing != null)
      {
      // Do Some Process with s.Thing
      .
      .
      .
      }
      }

      Which is a lot cleaner! ...and this isn't even considering a method that returns a value of some sort. Opinions?

      - I would love to change the world, but they won’t give me the source code.

      P Offline
      P Offline
      PIEBALDconsult
      wrote on last edited by
      #2

      I agree. I will use multiple returns when doing otherwise would be less readable or less efficient, but otherwise I prefer one return. And I don't listen to anyone who says that a throw or a yield is the same as a return.

      F 1 Reply Last reply
      0
      • F Forogar

        I have just had a heated argument cage fight lively discussion with some of my team members about ReSharper's suggestion of refactoring code to replace nested ifs with a series of multiple early return statements. This caused horribly messy code that ReSharper actually described in it's help as "more readable"! Using a version of their example code (which is a lot simpler than the actual code in question):

        void SomeFunction(SomeThing s)
        {
        if (s != null)
        {
        if (s.Thing != null)
        {
        // Do Some Process with s.Thing
        .
        .
        .
        }
        }
        }

        becomes:

        void SomeFunction(SomeThing s)
        {
        if (s == null) return
        if (s.Thing == null) return;
        // Do Some Process with s.Thing
        .
        .
        .
        }

        This makes a complete hash of the natural flow of the code and introduces an execution statement (return) on the same line as the "if" which is bad coding practice, and then does it again, and then drops though to do some actual processing. If I was to refactor the code it would do this:

        void SomeFunction(SomeThing s)
        {
        if (s != null && s.Thing != null)
        {
        // Do Some Process with s.Thing
        .
        .
        .
        }
        }

        or possibly:

        void SomeFunction(SomeThing s)
        {
        if (s?.Thing != null)
        {
        // Do Some Process with s.Thing
        .
        .
        .
        }
        }

        Which is a lot cleaner! ...and this isn't even considering a method that returns a value of some sort. Opinions?

        - I would love to change the world, but they won’t give me the source code.

        L Offline
        L Offline
        Lost User
        wrote on last edited by
        #3

        I actually sometimes throw exceptions instead of just return and yes I prefer strongly the early returns. It's more modular than the nested hell of mess. Especially when multiple people add crap within a particular branch of ifs. I looked at the source code of DOOM. John Carmack uses early returns. That should settle it.

        C 1 Reply Last reply
        0
        • P PIEBALDconsult

          I agree. I will use multiple returns when doing otherwise would be less readable or less efficient, but otherwise I prefer one return. And I don't listen to anyone who says that a throw or a yield is the same as a return.

          F Offline
          F Offline
          Forogar
          wrote on last edited by
          #4

          How about throwing exceptions?

          - I would love to change the world, but they won’t give me the source code.

          1 Reply Last reply
          0
          • F Forogar

            I have just had a heated argument cage fight lively discussion with some of my team members about ReSharper's suggestion of refactoring code to replace nested ifs with a series of multiple early return statements. This caused horribly messy code that ReSharper actually described in it's help as "more readable"! Using a version of their example code (which is a lot simpler than the actual code in question):

            void SomeFunction(SomeThing s)
            {
            if (s != null)
            {
            if (s.Thing != null)
            {
            // Do Some Process with s.Thing
            .
            .
            .
            }
            }
            }

            becomes:

            void SomeFunction(SomeThing s)
            {
            if (s == null) return
            if (s.Thing == null) return;
            // Do Some Process with s.Thing
            .
            .
            .
            }

            This makes a complete hash of the natural flow of the code and introduces an execution statement (return) on the same line as the "if" which is bad coding practice, and then does it again, and then drops though to do some actual processing. If I was to refactor the code it would do this:

            void SomeFunction(SomeThing s)
            {
            if (s != null && s.Thing != null)
            {
            // Do Some Process with s.Thing
            .
            .
            .
            }
            }

            or possibly:

            void SomeFunction(SomeThing s)
            {
            if (s?.Thing != null)
            {
            // Do Some Process with s.Thing
            .
            .
            .
            }
            }

            Which is a lot cleaner! ...and this isn't even considering a method that returns a value of some sort. Opinions?

            - I would love to change the world, but they won’t give me the source code.

            S Offline
            S Offline
            Simon_Whale
            wrote on last edited by
            #5

            That is the best advice I have seen from re-sharper and I've seen it show some bad alternatives one was and only know it was re-sharper as the Dev stated it was what re-sharper advised him to use

            public bool someMethod(int valueA, bool Valueb, bool ValueC)
            {
            var output = DoSomethingElse(ValueA, ValueB) ? RunAnotherMethod(ValueA, output.SOmething) :
            DoAnotherMethod(ValueB, ValueC) ? RunAthoerMethod(ValueB, output.SomethingElse) :
            DoYetAnotherMethod(ValueA, ValueC) RunAnotherMethod(DoYetAnotherMethod(ValueA, ValueC), output.AgainSomethingElse);
            }

            This horrid IF ELSE statement contains 16 if's Footnote: I don't like using Resharper

            Every day, thousands of innocent plants are killed by vegetarians. Help end the violence EAT BACON

            F 1 Reply Last reply
            0
            • S Simon_Whale

              That is the best advice I have seen from re-sharper and I've seen it show some bad alternatives one was and only know it was re-sharper as the Dev stated it was what re-sharper advised him to use

              public bool someMethod(int valueA, bool Valueb, bool ValueC)
              {
              var output = DoSomethingElse(ValueA, ValueB) ? RunAnotherMethod(ValueA, output.SOmething) :
              DoAnotherMethod(ValueB, ValueC) ? RunAthoerMethod(ValueB, output.SomethingElse) :
              DoYetAnotherMethod(ValueA, ValueC) RunAnotherMethod(DoYetAnotherMethod(ValueA, ValueC), output.AgainSomethingElse);
              }

              This horrid IF ELSE statement contains 16 if's Footnote: I don't like using Resharper

              Every day, thousands of innocent plants are killed by vegetarians. Help end the violence EAT BACON

              F Offline
              F Offline
              Forogar
              wrote on last edited by
              #6

              Layout can help a lot:

              public bool someMethod(int valueA, bool Valueb, bool ValueC)
              {
              var output = DoSomethingElse(ValueA, ValueB)
              ? RunAnotherMethod(ValueA, output.SOmething)
              : DoAnotherMethod(ValueB, ValueC)
              ? RunAthoerMethod(ValueB, output.SomethingElse)
              : DoYetAnotherMethod(ValueA, ValueC)
              ...etc.
              }

              - I would love to change the world, but they won’t give me the source code.

              C 1 Reply Last reply
              0
              • F Forogar

                I have just had a heated argument cage fight lively discussion with some of my team members about ReSharper's suggestion of refactoring code to replace nested ifs with a series of multiple early return statements. This caused horribly messy code that ReSharper actually described in it's help as "more readable"! Using a version of their example code (which is a lot simpler than the actual code in question):

                void SomeFunction(SomeThing s)
                {
                if (s != null)
                {
                if (s.Thing != null)
                {
                // Do Some Process with s.Thing
                .
                .
                .
                }
                }
                }

                becomes:

                void SomeFunction(SomeThing s)
                {
                if (s == null) return
                if (s.Thing == null) return;
                // Do Some Process with s.Thing
                .
                .
                .
                }

                This makes a complete hash of the natural flow of the code and introduces an execution statement (return) on the same line as the "if" which is bad coding practice, and then does it again, and then drops though to do some actual processing. If I was to refactor the code it would do this:

                void SomeFunction(SomeThing s)
                {
                if (s != null && s.Thing != null)
                {
                // Do Some Process with s.Thing
                .
                .
                .
                }
                }

                or possibly:

                void SomeFunction(SomeThing s)
                {
                if (s?.Thing != null)
                {
                // Do Some Process with s.Thing
                .
                .
                .
                }
                }

                Which is a lot cleaner! ...and this isn't even considering a method that returns a value of some sort. Opinions?

                - I would love to change the world, but they won’t give me the source code.

                L Offline
                L Offline
                Lost User
                wrote on last edited by
                #7

                Ah yes, I found it. Here some food for thought: [The Fail-Fast Principle in Software Development](https://dzone.com/articles/fail-fast-principle-in-software-development) [https://www.martinfowler.com/ieeeSoftware/failFast.pdf\](https://www.martinfowler.com/ieeeSoftware/failFast.pdf)

                1 Reply Last reply
                0
                • F Forogar

                  I have just had a heated argument cage fight lively discussion with some of my team members about ReSharper's suggestion of refactoring code to replace nested ifs with a series of multiple early return statements. This caused horribly messy code that ReSharper actually described in it's help as "more readable"! Using a version of their example code (which is a lot simpler than the actual code in question):

                  void SomeFunction(SomeThing s)
                  {
                  if (s != null)
                  {
                  if (s.Thing != null)
                  {
                  // Do Some Process with s.Thing
                  .
                  .
                  .
                  }
                  }
                  }

                  becomes:

                  void SomeFunction(SomeThing s)
                  {
                  if (s == null) return
                  if (s.Thing == null) return;
                  // Do Some Process with s.Thing
                  .
                  .
                  .
                  }

                  This makes a complete hash of the natural flow of the code and introduces an execution statement (return) on the same line as the "if" which is bad coding practice, and then does it again, and then drops though to do some actual processing. If I was to refactor the code it would do this:

                  void SomeFunction(SomeThing s)
                  {
                  if (s != null && s.Thing != null)
                  {
                  // Do Some Process with s.Thing
                  .
                  .
                  .
                  }
                  }

                  or possibly:

                  void SomeFunction(SomeThing s)
                  {
                  if (s?.Thing != null)
                  {
                  // Do Some Process with s.Thing
                  .
                  .
                  .
                  }
                  }

                  Which is a lot cleaner! ...and this isn't even considering a method that returns a value of some sort. Opinions?

                  - I would love to change the world, but they won’t give me the source code.

                  OriginalGriffO Offline
                  OriginalGriffO Offline
                  OriginalGriff
                  wrote on last edited by
                  #8

                  Under most circumstances, a single return is best. But ... I much prefer this:

                  void MyFunc ()
                  {
                  if (!ValidateUserInput(ATextBox.Text))
                  {
                  TellUserAboutTheProblem();
                  return;
                  }
                  if (!ValidateUserInput(AnotherTextBox.Text))
                  {
                  TellUserAboutTheProblem();
                  return;
                  }
                  ...
                  }

                  To this:

                  void MyFunc ()
                  {
                  if (!ValidateUserInput(ATextBox.Text))
                  {
                  TellUserAboutTheProblem();
                  }
                  else if (!ValidateUserInput(AnotherTextBox.Text))
                  {
                  TellUserAboutTheProblem();
                  }
                  else
                  {
                  ...
                  }
                  }

                  And sometimes the best thing to do is just return, particularly from a nested loop:

                  for (int i = 0; i < 100; i++)
                  {
                  for (int j = 0; j < 100; j++)
                  {
                  if (myArray[i, j] == value) return true;
                  }
                  }
                  return false;

                  Any other mechanism is just making it more complicated, not less.

                  Sent from my Amstrad PC 1640 Never throw anything away, Griff Bad command or file name. Bad, bad command! Sit! Stay! Staaaay... AntiTwitter: @DalekDave is now a follower!

                  "I have no idea what I did, but I'm taking full credit for it." - ThisOldTony
                  "Common sense is so rare these days, it should be classified as a super power" - Random T-shirt

                  J M M 3 Replies Last reply
                  0
                  • F Forogar

                    I have just had a heated argument cage fight lively discussion with some of my team members about ReSharper's suggestion of refactoring code to replace nested ifs with a series of multiple early return statements. This caused horribly messy code that ReSharper actually described in it's help as "more readable"! Using a version of their example code (which is a lot simpler than the actual code in question):

                    void SomeFunction(SomeThing s)
                    {
                    if (s != null)
                    {
                    if (s.Thing != null)
                    {
                    // Do Some Process with s.Thing
                    .
                    .
                    .
                    }
                    }
                    }

                    becomes:

                    void SomeFunction(SomeThing s)
                    {
                    if (s == null) return
                    if (s.Thing == null) return;
                    // Do Some Process with s.Thing
                    .
                    .
                    .
                    }

                    This makes a complete hash of the natural flow of the code and introduces an execution statement (return) on the same line as the "if" which is bad coding practice, and then does it again, and then drops though to do some actual processing. If I was to refactor the code it would do this:

                    void SomeFunction(SomeThing s)
                    {
                    if (s != null && s.Thing != null)
                    {
                    // Do Some Process with s.Thing
                    .
                    .
                    .
                    }
                    }

                    or possibly:

                    void SomeFunction(SomeThing s)
                    {
                    if (s?.Thing != null)
                    {
                    // Do Some Process with s.Thing
                    .
                    .
                    .
                    }
                    }

                    Which is a lot cleaner! ...and this isn't even considering a method that returns a value of some sort. Opinions?

                    - I would love to change the world, but they won’t give me the source code.

                    J Offline
                    J Offline
                    Jorgen Andersson
                    wrote on last edited by
                    #9

                    That would depend on whether I return a value or not. On a related note, K&R or Allman?

                    Wrong is evil and must be defeated. - Jeff Ello

                    OriginalGriffO 1 Reply Last reply
                    0
                    • J Jorgen Andersson

                      That would depend on whether I return a value or not. On a related note, K&R or Allman?

                      Wrong is evil and must be defeated. - Jeff Ello

                      OriginalGriffO Offline
                      OriginalGriffO Offline
                      OriginalGriff
                      wrote on last edited by
                      #10

                      Whitesmiths.

                      Sent from my Amstrad PC 1640 Never throw anything away, Griff Bad command or file name. Bad, bad command! Sit! Stay! Staaaay... AntiTwitter: @DalekDave is now a follower!

                      "I have no idea what I did, but I'm taking full credit for it." - ThisOldTony
                      "Common sense is so rare these days, it should be classified as a super power" - Random T-shirt

                      J J 2 Replies Last reply
                      0
                      • OriginalGriffO OriginalGriff

                        Whitesmiths.

                        Sent from my Amstrad PC 1640 Never throw anything away, Griff Bad command or file name. Bad, bad command! Sit! Stay! Staaaay... AntiTwitter: @DalekDave is now a follower!

                        J Offline
                        J Offline
                        Jorgen Andersson
                        wrote on last edited by
                        #11

                        I did expect that answer. :laugh:

                        Wrong is evil and must be defeated. - Jeff Ello

                        1 Reply Last reply
                        0
                        • OriginalGriffO OriginalGriff

                          Under most circumstances, a single return is best. But ... I much prefer this:

                          void MyFunc ()
                          {
                          if (!ValidateUserInput(ATextBox.Text))
                          {
                          TellUserAboutTheProblem();
                          return;
                          }
                          if (!ValidateUserInput(AnotherTextBox.Text))
                          {
                          TellUserAboutTheProblem();
                          return;
                          }
                          ...
                          }

                          To this:

                          void MyFunc ()
                          {
                          if (!ValidateUserInput(ATextBox.Text))
                          {
                          TellUserAboutTheProblem();
                          }
                          else if (!ValidateUserInput(AnotherTextBox.Text))
                          {
                          TellUserAboutTheProblem();
                          }
                          else
                          {
                          ...
                          }
                          }

                          And sometimes the best thing to do is just return, particularly from a nested loop:

                          for (int i = 0; i < 100; i++)
                          {
                          for (int j = 0; j < 100; j++)
                          {
                          if (myArray[i, j] == value) return true;
                          }
                          }
                          return false;

                          Any other mechanism is just making it more complicated, not less.

                          Sent from my Amstrad PC 1640 Never throw anything away, Griff Bad command or file name. Bad, bad command! Sit! Stay! Staaaay... AntiTwitter: @DalekDave is now a follower!

                          J Offline
                          J Offline
                          Jorgen Andersson
                          wrote on last edited by
                          #12

                          That's the longer and much better version of what i couldn't bother to write down. :)

                          Wrong is evil and must be defeated. - Jeff Ello

                          1 Reply Last reply
                          0
                          • F Forogar

                            I have just had a heated argument cage fight lively discussion with some of my team members about ReSharper's suggestion of refactoring code to replace nested ifs with a series of multiple early return statements. This caused horribly messy code that ReSharper actually described in it's help as "more readable"! Using a version of their example code (which is a lot simpler than the actual code in question):

                            void SomeFunction(SomeThing s)
                            {
                            if (s != null)
                            {
                            if (s.Thing != null)
                            {
                            // Do Some Process with s.Thing
                            .
                            .
                            .
                            }
                            }
                            }

                            becomes:

                            void SomeFunction(SomeThing s)
                            {
                            if (s == null) return
                            if (s.Thing == null) return;
                            // Do Some Process with s.Thing
                            .
                            .
                            .
                            }

                            This makes a complete hash of the natural flow of the code and introduces an execution statement (return) on the same line as the "if" which is bad coding practice, and then does it again, and then drops though to do some actual processing. If I was to refactor the code it would do this:

                            void SomeFunction(SomeThing s)
                            {
                            if (s != null && s.Thing != null)
                            {
                            // Do Some Process with s.Thing
                            .
                            .
                            .
                            }
                            }

                            or possibly:

                            void SomeFunction(SomeThing s)
                            {
                            if (s?.Thing != null)
                            {
                            // Do Some Process with s.Thing
                            .
                            .
                            .
                            }
                            }

                            Which is a lot cleaner! ...and this isn't even considering a method that returns a value of some sort. Opinions?

                            - I would love to change the world, but they won’t give me the source code.

                            L Offline
                            L Offline
                            Lost User
                            wrote on last edited by
                            #13

                            sort of depends on what I call the "story" if the story says "if X is null or is [example] an empty list then stop processing" = in my mind early return. if the story says "do thus-ad-that to the contents of X" - to save on errors naturally check X for null/empty makes sense - then of course it's if (X != null) ...

                            Forogar wrote:

                            ntroduces an execution statement (return) on the same line as the "if" which is bad coding practice

                            ?????????????? bad coding practice ????????????????? huh? Nothing is wrong, nor bad, nor unsafe with that code, it is perfectly good code. you confuse practice with style: style is subjective, "bad practice" increases the chance for error or failure. personally I dislike code that runs too many pages, so I often put short single statements and opening braces on the same line as the if (), while () etc. (Coming from K&R style C background.) and I like .... AND SO, before you say it, I'll say it for you: --- you think my style is ugly. Well guess what: ---- I think your style is ugly. AND WHO CARES!! 1. it's the code that matters, NOT THE STYLE, 2. different style IS NOT BAD PRACTICE weather you like it or not. (please don't bring up "readability" bullshit, I find more compact more readable, please don't bring up "industry standard" - my K&R background, and I'm not the only one doing it that way.) 3. you can have the editor re-format [to your preferred] style on one keystroke, so even more WHO CARES. nuff said.

                            Message Signature (Click to edit ->)

                            A 1 Reply Last reply
                            0
                            • F Forogar

                              I have just had a heated argument cage fight lively discussion with some of my team members about ReSharper's suggestion of refactoring code to replace nested ifs with a series of multiple early return statements. This caused horribly messy code that ReSharper actually described in it's help as "more readable"! Using a version of their example code (which is a lot simpler than the actual code in question):

                              void SomeFunction(SomeThing s)
                              {
                              if (s != null)
                              {
                              if (s.Thing != null)
                              {
                              // Do Some Process with s.Thing
                              .
                              .
                              .
                              }
                              }
                              }

                              becomes:

                              void SomeFunction(SomeThing s)
                              {
                              if (s == null) return
                              if (s.Thing == null) return;
                              // Do Some Process with s.Thing
                              .
                              .
                              .
                              }

                              This makes a complete hash of the natural flow of the code and introduces an execution statement (return) on the same line as the "if" which is bad coding practice, and then does it again, and then drops though to do some actual processing. If I was to refactor the code it would do this:

                              void SomeFunction(SomeThing s)
                              {
                              if (s != null && s.Thing != null)
                              {
                              // Do Some Process with s.Thing
                              .
                              .
                              .
                              }
                              }

                              or possibly:

                              void SomeFunction(SomeThing s)
                              {
                              if (s?.Thing != null)
                              {
                              // Do Some Process with s.Thing
                              .
                              .
                              .
                              }
                              }

                              Which is a lot cleaner! ...and this isn't even considering a method that returns a value of some sort. Opinions?

                              - I would love to change the world, but they won’t give me the source code.

                              C Offline
                              C Offline
                              CodeWraith
                              wrote on last edited by
                              #14

                              Simply do it how you think it's ok. As long as it's only a question of preferences, I see no salomonian compromise. Why? Let's assume you can keep the method short and to the point, with only few conditions that 'spaghettify' the code. In that case I hope that all people involved are intelligent enough to read the code, despite it not having their preferred format. In the other case, hopefully rare, where you have no choice and the method gets a little longer and has many if/else conditions, then religiously trying to put it in a certain format will most probably 'spaghettify' it even more and make it even harder to understand. That's why I always prefer good judgement over religiously (meaning thoughtlessly) enforcing 'standards' at all cost. Whose standards? Those of the team as a common ground or those of a few beaurocrats who never find an end? Been there, done that. I was once in charge of writing the 'rulebook' and code review. It kept getting longer every week. Rules, more rules, exceptions to the rules and then of course alternate rules. In the end it was always one or two people who turned the code review into a discussion about their personal preferences while the rest of the team started to ignore the whole worthless circus and automatically getting the buerocrats off their backs by giving them so much to freak out about that even they could not keep up with inventing more 'standards'. If those idiots want someone to type exactly what they want, why don't they get themselves a secretary?

                              I have lived with several Zen masters - all of them were cats. His last invention was an evil Lasagna. It didn't kill anyone, and it actually tasted pretty good.

                              1 Reply Last reply
                              0
                              • F Forogar

                                I have just had a heated argument cage fight lively discussion with some of my team members about ReSharper's suggestion of refactoring code to replace nested ifs with a series of multiple early return statements. This caused horribly messy code that ReSharper actually described in it's help as "more readable"! Using a version of their example code (which is a lot simpler than the actual code in question):

                                void SomeFunction(SomeThing s)
                                {
                                if (s != null)
                                {
                                if (s.Thing != null)
                                {
                                // Do Some Process with s.Thing
                                .
                                .
                                .
                                }
                                }
                                }

                                becomes:

                                void SomeFunction(SomeThing s)
                                {
                                if (s == null) return
                                if (s.Thing == null) return;
                                // Do Some Process with s.Thing
                                .
                                .
                                .
                                }

                                This makes a complete hash of the natural flow of the code and introduces an execution statement (return) on the same line as the "if" which is bad coding practice, and then does it again, and then drops though to do some actual processing. If I was to refactor the code it would do this:

                                void SomeFunction(SomeThing s)
                                {
                                if (s != null && s.Thing != null)
                                {
                                // Do Some Process with s.Thing
                                .
                                .
                                .
                                }
                                }

                                or possibly:

                                void SomeFunction(SomeThing s)
                                {
                                if (s?.Thing != null)
                                {
                                // Do Some Process with s.Thing
                                .
                                .
                                .
                                }
                                }

                                Which is a lot cleaner! ...and this isn't even considering a method that returns a value of some sort. Opinions?

                                - I would love to change the world, but they won’t give me the source code.

                                L Offline
                                L Offline
                                Lost User
                                wrote on last edited by
                                #15

                                The (professional) opinion is that humans work better with "positive" statements vs deciphering compound negatives. I will even resort to: if ( a && b && c ){ // continue. } else { return; } ... for myself. ("Early returns" probably run contrary to the notion of "diving into the code"; but since code should not run more than "a page", "diving" implies a bigger problem).

                                "(I) am amazed to see myself here rather than there ... now rather than then". ― Blaise Pascal

                                1 Reply Last reply
                                0
                                • F Forogar

                                  I have just had a heated argument cage fight lively discussion with some of my team members about ReSharper's suggestion of refactoring code to replace nested ifs with a series of multiple early return statements. This caused horribly messy code that ReSharper actually described in it's help as "more readable"! Using a version of their example code (which is a lot simpler than the actual code in question):

                                  void SomeFunction(SomeThing s)
                                  {
                                  if (s != null)
                                  {
                                  if (s.Thing != null)
                                  {
                                  // Do Some Process with s.Thing
                                  .
                                  .
                                  .
                                  }
                                  }
                                  }

                                  becomes:

                                  void SomeFunction(SomeThing s)
                                  {
                                  if (s == null) return
                                  if (s.Thing == null) return;
                                  // Do Some Process with s.Thing
                                  .
                                  .
                                  .
                                  }

                                  This makes a complete hash of the natural flow of the code and introduces an execution statement (return) on the same line as the "if" which is bad coding practice, and then does it again, and then drops though to do some actual processing. If I was to refactor the code it would do this:

                                  void SomeFunction(SomeThing s)
                                  {
                                  if (s != null && s.Thing != null)
                                  {
                                  // Do Some Process with s.Thing
                                  .
                                  .
                                  .
                                  }
                                  }

                                  or possibly:

                                  void SomeFunction(SomeThing s)
                                  {
                                  if (s?.Thing != null)
                                  {
                                  // Do Some Process with s.Thing
                                  .
                                  .
                                  .
                                  }
                                  }

                                  Which is a lot cleaner! ...and this isn't even considering a method that returns a value of some sort. Opinions?

                                  - I would love to change the world, but they won’t give me the source code.

                                  Z Offline
                                  Z Offline
                                  ZurdoDev
                                  wrote on last edited by
                                  #16

                                  I believe I am in the minority on this but I prefer one single return statement. Multiple returns adds unnecessary confusion. For example, if you put a breakpoint near the end of a function and it never hits it may be because of the early returns so you have to go hunting around to find out what's going on. One return.

                                  Social Media - A platform that makes it easier for the crazies to find each other. Everyone is born right handed. Only the strongest overcome it. Fight for left-handed rights and hand equality.

                                  L C M 3 Replies Last reply
                                  0
                                  • L Lost User

                                    I actually sometimes throw exceptions instead of just return and yes I prefer strongly the early returns. It's more modular than the nested hell of mess. Especially when multiple people add crap within a particular branch of ifs. I looked at the source code of DOOM. John Carmack uses early returns. That should settle it.

                                    C Offline
                                    C Offline
                                    CodeWraith
                                    wrote on last edited by
                                    #17

                                    Urban Cricket wrote:

                                    John Carmack uses early returns.

                                    Looking at my bookshelf: The Graphics Programming Black Book, by Michael Abrash and with a foreword by John Carmack. Fearsomely thick tome, full of how to write fast graphics code. That kind of processor cycle counting on a 386 or 486 is outdated, while the algorithmic optimizations remain as current as ever, especially if you are able to delegate them to the graphics processor. Early returns as a way to waste no processor cycle too much in a function is only rarely important anymore.

                                    I have lived with several Zen masters - all of them were cats. His last invention was an evil Lasagna. It didn't kill anyone, and it actually tasted pretty good.

                                    L S 2 Replies Last reply
                                    0
                                    • F Forogar

                                      Layout can help a lot:

                                      public bool someMethod(int valueA, bool Valueb, bool ValueC)
                                      {
                                      var output = DoSomethingElse(ValueA, ValueB)
                                      ? RunAnotherMethod(ValueA, output.SOmething)
                                      : DoAnotherMethod(ValueB, ValueC)
                                      ? RunAthoerMethod(ValueB, output.SomethingElse)
                                      : DoYetAnotherMethod(ValueA, ValueC)
                                      ...etc.
                                      }

                                      - I would love to change the world, but they won’t give me the source code.

                                      C Offline
                                      C Offline
                                      CodeWraith
                                      wrote on last edited by
                                      #18

                                      Still: Yuck!

                                      I have lived with several Zen masters - all of them were cats. His last invention was an evil Lasagna. It didn't kill anyone, and it actually tasted pretty good.

                                      C 1 Reply Last reply
                                      0
                                      • Z ZurdoDev

                                        I believe I am in the minority on this but I prefer one single return statement. Multiple returns adds unnecessary confusion. For example, if you put a breakpoint near the end of a function and it never hits it may be because of the early returns so you have to go hunting around to find out what's going on. One return.

                                        Social Media - A platform that makes it easier for the crazies to find each other. Everyone is born right handed. Only the strongest overcome it. Fight for left-handed rights and hand equality.

                                        L Offline
                                        L Offline
                                        Lost User
                                        wrote on last edited by
                                        #19

                                        Putting a break point that does not get hit where you expect it to only implies that one does not understand the program flow; not that there could be "too many returns". Without a try-catch block everywhere, every exception amounts to an "early return". Also, "early returns" facilitate the returning of different "condition codes"; instead of "tramping" them along. A la IBM; 0 - Good 4 - Good with conditions 8 - Warning 16 - Oh Oh ...

                                        "(I) am amazed to see myself here rather than there ... now rather than then". ― Blaise Pascal

                                        Z 1 Reply Last reply
                                        0
                                        • L Lost User

                                          Putting a break point that does not get hit where you expect it to only implies that one does not understand the program flow; not that there could be "too many returns". Without a try-catch block everywhere, every exception amounts to an "early return". Also, "early returns" facilitate the returning of different "condition codes"; instead of "tramping" them along. A la IBM; 0 - Good 4 - Good with conditions 8 - Warning 16 - Oh Oh ...

                                          "(I) am amazed to see myself here rather than there ... now rather than then". ― Blaise Pascal

                                          Z Offline
                                          Z Offline
                                          ZurdoDev
                                          wrote on last edited by
                                          #20

                                          Gerry Schmitz wrote:

                                          only implies that one does not understand the program flow;

                                          Exactly! :-D But it's very easy to assume the code is failing down near the bottom of the function and therefore you put the breakpoint there because you don't want to step through all of it.

                                          Social Media - A platform that makes it easier for the crazies to find each other. Everyone is born right handed. Only the strongest overcome it. Fight for left-handed rights and hand equality.

                                          1 Reply Last reply
                                          0
                                          Reply
                                          • Reply as topic
                                          Log in to reply
                                          • Oldest to Newest
                                          • Newest to Oldest
                                          • Most Votes


                                          • Login

                                          • Don't have an account? Register

                                          • Login or register to search.
                                          • First post
                                            Last post
                                          0
                                          • Categories
                                          • Recent
                                          • Tags
                                          • Popular
                                          • World
                                          • Users
                                          • Groups