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. Would this pass code review where you are?

Would this pass code review where you are?

Scheduled Pinned Locked Moved The Lounge
csharpcomquestiondiscussioncode-review
46 Posts 34 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.
  • 5 5teveH

    You need to tell the author he's in the wrong job. He needs to find something where logic and common sense isn't required. Management, maybe? ;)

    J Offline
    J Offline
    John Torjo
    wrote on last edited by
    #37

    He would probably ace politics...

    1 Reply Last reply
    0
    • OriginalGriffO OriginalGriff

      I didn't write it, but ... the author thinks this is a good idea that should be allowed in C# code:

      getSeries()
      {

      if (flag == pass) goto ep;

      for i1 ...
      ...
      for i2 ...
      ...
      for i3 ...
      ...
      for i4 ...
      .
      .
      .
      for ...
      {
      ...
      return;
      ep;
      }
      }
      }
      }//last for

      }// end of getSeries()

      Me? I'm not a fan. Your thoughts - and remember this is the Lounge? :laugh:

      "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 AntiTwitter: @DalekDave is now a follower!

      D Offline
      D Offline
      Davyd McColl
      wrote on last edited by
      #38

      No, simply for nesting / length. I'm sure well-named functions could be pulled out, which would decrease the cognitive load on the reader, especially someone new to the code, which includes the original author in about a month or two.

      ------------------------------------------------ If you say that getting the money is the most important thing You will spend your life completely wasting your time You will be doing things you don't like doing In order to go on living That is, to go on doing things you don't like doing Which is stupid. - Alan Watts https://www.youtube.com/watch?v=-gXTZM\_uPMY

      1 Reply Last reply
      0
      • OriginalGriffO OriginalGriff

        I didn't write it, but ... the author thinks this is a good idea that should be allowed in C# code:

        getSeries()
        {

        if (flag == pass) goto ep;

        for i1 ...
        ...
        for i2 ...
        ...
        for i3 ...
        ...
        for i4 ...
        .
        .
        .
        for ...
        {
        ...
        return;
        ep;
        }
        }
        }
        }//last for

        }// end of getSeries()

        Me? I'm not a fan. Your thoughts - and remember this is the Lounge? :laugh:

        "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 AntiTwitter: @DalekDave is now a follower!

        M Offline
        M Offline
        Martin ISDN
        wrote on last edited by
        #39

        Pascal, for instance, will not allow you to hyperspace into a scope like that. for good reason. the only useful scenario for this "technique" is where you have the expressions after the closing brace of the inner loops. that way either you execute all the looping which creates some multi dimensional computation or you jump with goto inside the most inner loop and execute a liner computation, once for every loop. but, in this case i fail to comprehend the reason for jumping at the tail of the loops. equally puzzled by the most inner return. this looks like is some kind of a trick interview question... it would be interesting to know what is the purpose of this "device"?

        1 Reply Last reply
        0
        • OriginalGriffO OriginalGriff

          I didn't write it, but ... the author thinks this is a good idea that should be allowed in C# code:

          getSeries()
          {

          if (flag == pass) goto ep;

          for i1 ...
          ...
          for i2 ...
          ...
          for i3 ...
          ...
          for i4 ...
          .
          .
          .
          for ...
          {
          ...
          return;
          ep;
          }
          }
          }
          }//last for

          }// end of getSeries()

          Me? I'm not a fan. Your thoughts - and remember this is the Lounge? :laugh:

          "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 AntiTwitter: @DalekDave is now a follower!

          M Offline
          M Offline
          Myron Dombrowski
          wrote on last edited by
          #40

          In four words: nope.

          1 Reply Last reply
          0
          • OriginalGriffO OriginalGriff

            I didn't write it, but ... the author thinks this is a good idea that should be allowed in C# code:

            getSeries()
            {

            if (flag == pass) goto ep;

            for i1 ...
            ...
            for i2 ...
            ...
            for i3 ...
            ...
            for i4 ...
            .
            .
            .
            for ...
            {
            ...
            return;
            ep;
            }
            }
            }
            }//last for

            }// end of getSeries()

            Me? I'm not a fan. Your thoughts - and remember this is the Lounge? :laugh:

            "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 AntiTwitter: @DalekDave is now a follower!

            B Offline
            B Offline
            Bruce Patin
            wrote on last edited by
            #41

            That was my very first program in the 1960s. It was a FORTRAN loop to calculate Chess moves. My Dad punched it into cards and had it evaluated, and said it would take more years to compute than there are stars in the Universe.

            1 Reply Last reply
            0
            • OriginalGriffO OriginalGriff

              I didn't write it, but ... the author thinks this is a good idea that should be allowed in C# code:

              getSeries()
              {

              if (flag == pass) goto ep;

              for i1 ...
              ...
              for i2 ...
              ...
              for i3 ...
              ...
              for i4 ...
              .
              .
              .
              for ...
              {
              ...
              return;
              ep;
              }
              }
              }
              }//last for

              }// end of getSeries()

              Me? I'm not a fan. Your thoughts - and remember this is the Lounge? :laugh:

              "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 AntiTwitter: @DalekDave is now a follower!

              N Offline
              N Offline
              NightPen
              wrote on last edited by
              #42

              In my career, I actually had to review a piece of code similar to this. My approach was to turn the code review into a teaching moment. I had the developer explain the problem they were trying to solve, what solutions they considered and why they chose this one. The developer ended up reworking the code and produced a much-improved solution. More importantly, though the developer never sent code like this in for code review again.

              1 Reply Last reply
              0
              • OriginalGriffO OriginalGriff

                I didn't write it, but ... the author thinks this is a good idea that should be allowed in C# code:

                getSeries()
                {

                if (flag == pass) goto ep;

                for i1 ...
                ...
                for i2 ...
                ...
                for i3 ...
                ...
                for i4 ...
                .
                .
                .
                for ...
                {
                ...
                return;
                ep;
                }
                }
                }
                }//last for

                }// end of getSeries()

                Me? I'm not a fan. Your thoughts - and remember this is the Lounge? :laugh:

                "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 AntiTwitter: @DalekDave is now a follower!

                F Offline
                F Offline
                Fabio Franco
                wrote on last edited by
                #43

                Not sure what I find worse, the amount of nested for's or the use of goto.

                To alcohol! The cause of, and solution to, all of life's problems - Homer Simpson ---- Our heads are round so our thoughts can change direction - Francis Picabia

                1 Reply Last reply
                0
                • R realJSOP

                  Without knowing the range of the for loops, and how many there are, I wouldn't recommend recursion, because it might overflow the stack.

                  ".45 ACP - because shooting twice is just silly" - JSOP, 2010
                  -----
                  You can never have too much ammo - unless you're swimming, or on fire. - JSOP, 2010
                  -----
                  When you pry the gun from my cold dead hands, be careful - the barrel will be very hot. - JSOP, 2013

                  F Offline
                  F Offline
                  Fabio Franco
                  wrote on last edited by
                  #44

                  Are you being ironic? Or do you actually think it's better to use nested fors explicitly, instead of passing a depth limit as a parameter of a recursive function?

                  To alcohol! The cause of, and solution to, all of life's problems - Homer Simpson ---- Our heads are round so our thoughts can change direction - Francis Picabia

                  R 1 Reply Last reply
                  0
                  • OriginalGriffO OriginalGriff

                    I didn't write it, but ... the author thinks this is a good idea that should be allowed in C# code:

                    getSeries()
                    {

                    if (flag == pass) goto ep;

                    for i1 ...
                    ...
                    for i2 ...
                    ...
                    for i3 ...
                    ...
                    for i4 ...
                    .
                    .
                    .
                    for ...
                    {
                    ...
                    return;
                    ep;
                    }
                    }
                    }
                    }//last for

                    }// end of getSeries()

                    Me? I'm not a fan. Your thoughts - and remember this is the Lounge? :laugh:

                    "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 AntiTwitter: @DalekDave is now a follower!

                    J Offline
                    J Offline
                    James Lonero
                    wrote on last edited by
                    #45

                    Just the nesting of loops makes me cringe. Ouch. I have written software for biotech companies creating chemistry applications, libraries, and drivers and see this often. Comparing, subtracting, etc. x,y,z points in one group of data to another group of data, we get this awful type of code. But, that’s what the customer wants us to do. And he wants it better, faster, and cheaper.

                    1 Reply Last reply
                    0
                    • F Fabio Franco

                      Are you being ironic? Or do you actually think it's better to use nested fors explicitly, instead of passing a depth limit as a parameter of a recursive function?

                      To alcohol! The cause of, and solution to, all of life's problems - Homer Simpson ---- Our heads are round so our thoughts can change direction - Francis Picabia

                      R Offline
                      R Offline
                      realJSOP
                      wrote on last edited by
                      #46
                      1. If you need to specify a depth limit, you have problems elsewhere in the implementation. 1) It's impossible for us to properly evaluate the code without knowing much more about it. Right now, everyone's just throwing darts...

                      ".45 ACP - because shooting twice is just silly" - JSOP, 2010
                      -----
                      You can never have too much ammo - unless you're swimming, or on fire. - JSOP, 2010
                      -----
                      When you pry the gun from my cold dead hands, be careful - the barrel will be very hot. - JSOP, 2013

                      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