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.
  • 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!

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

    OriginalGriff wrote:

    code review

    Sorry, what does that mean? :confused:

    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!

      Greg UtasG Offline
      Greg UtasG Offline
      Greg Utas
      wrote on last edited by
      #5

      Did you mean

      ep: return;

      to assuage those who believe a function should only return in one place?

      Robust Services Core | Software Techniques for Lemmings | Articles
      The fox knows many things, but the hedgehog knows one big thing.

      <p><a href="https://github.com/GregUtas/robust-services-core/blob/master/README.md">Robust Services Core</a>
      <em>The fox knows many things, but the hedgehog knows one big thing.</em></p>

      C 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
        Daniel Pfeffer
        wrote on last edited by
        #6

        If I had anything to do with it, the entity responsible would be having his/her/its exit interview ASAP. :wtf: :omg: X| A goto to a single error handler/exit point may be excusable. A goto into an inner loop should be cause for termination with extreme prejudice, pour encourager les autres!

        Freedom is the freedom to say that two plus two make four. If that is granted, all else follows. -- 6079 Smith W.

        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!

          R Offline
          R Offline
          RickZeeland
          wrote on last edited by
          #7

          Looks like a fruity loops kind of guy :-\

          1 Reply Last reply
          0
          • Greg UtasG Greg Utas

            Did you mean

            ep: return;

            to assuage those who believe a function should only return in one place?

            Robust Services Core | Software Techniques for Lemmings | Articles
            The fox knows many things, but the hedgehog knows one big thing.

            C Offline
            C Offline
            CPallini
            wrote on last edited by
            #8

            The ep label is after the return statement, in the OP (pseudo) code.

            Greg UtasG 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!

              C Offline
              C Offline
              CPallini
              wrote on last edited by
              #9

              It comes from an old, working C++ project, which, in turn comes from an older, working C project, which, in turn, comes from an ancient, ugly, not-working assembly project.

              1 Reply Last reply
              0
              • C CPallini

                The ep label is after the return statement, in the OP (pseudo) code.

                Greg UtasG Offline
                Greg UtasG Offline
                Greg Utas
                wrote on last edited by
                #10

                That's why I asked. Jumping into the middle of a for loop makes no sense to me.

                Robust Services Core | Software Techniques for Lemmings | Articles
                The fox knows many things, but the hedgehog knows one big thing.

                <p><a href="https://github.com/GregUtas/robust-services-core/blob/master/README.md">Robust Services Core</a>
                <em>The fox knows many things, but the hedgehog knows one big thing.</em></p>

                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
                  MarkB123
                  wrote on last edited by
                  #11

                  The first and most important question - does it work? The second question, how long ago was it written and what were the time-constraints and pressures of the project? It's easy to stand there and criticize other peoples work. Pretty sure no matter what you produce code-wise there will be better ways to do it either now or in the future. I never understand why people feel the need to post things like this - I'd guess some underlying insecurity issues...

                  OriginalGriffO 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!

                    Richard DeemingR Offline
                    Richard DeemingR Offline
                    Richard Deeming
                    wrote on last edited by
                    #12

                    Certainly not. What would the loop variables be set to when you jumped into the middle of the loop? :wtf: You can sort-of mimic something similar with an async method, by putting a yield return; in the inner loop. But IIRC, the compiler has to rewrite that as a state machine, because jumping into the middle of a loop just doesn't make any sense.


                    "These people looked deep within my soul and assigned me a number based on the order in which I joined." - Homer

                    "These people looked deep within my soul and assigned me a number based on the order in which I joined" - Homer

                    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!

                      W Offline
                      W Offline
                      W Balboos GHB
                      wrote on last edited by
                      #13

                      I review my own code - so I suppose, if I wrote it, it would pass. But I didn't - looks more like something one of our overseas contractors would do.

                      Ravings en masse^

                      "The difference between genius and stupidity is that genius has its limits." - Albert Einstein

                      "If you are searching for perfection in others, then you seek disappointment. If you seek perfection in yourself, then you will find failure." - Balboos HaGadol Mar 2010

                      1 Reply Last reply
                      0
                      • M MarkB123

                        The first and most important question - does it work? The second question, how long ago was it written and what were the time-constraints and pressures of the project? It's easy to stand there and criticize other peoples work. Pretty sure no matter what you produce code-wise there will be better ways to do it either now or in the future. I never understand why people feel the need to post things like this - I'd guess some underlying insecurity issues...

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

                        Apparently so, in C++. In C# (his target language) no - the label is out of scope of the goto. It was apparently written in 2006 ... and in my opinion you shouldn't try to perpetuate code crimes into new languages. To be honest, any coder who thinks nesting loops 5+ deep is a good idea probably needs a lesson in exponential growth ... :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!

                        "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

                        F 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!

                          Sander RosselS Offline
                          Sander RosselS Offline
                          Sander Rossel
                          wrote on last edited by
                          #15

                          It's this new cool style of programming named For-Oriented Programming, or FOP for short. I think I'll try it out on my next project :thumbsup:

                          Best, Sander Azure Serverless Succinctly Migrating Applications to the Cloud with Azure arrgh.js - Bringing LINQ to JavaScript

                          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
                            jsc42
                            wrote on last edited by
                            #16

                            Would be a good candidate for The Weird And The Wonderful forum, rather than the Lounge. I'm surprised it compiled. Shouldn't 'ep;' be 'ep:;'? I don't know since it is 43 years since I last wrote a goto statement (excluding for old languages that needed 'ON ERROR GOTO ..', and that was as recent as the late 1990s).

                            C 1 Reply Last reply
                            0
                            • J jsc42

                              Would be a good candidate for The Weird And The Wonderful forum, rather than the Lounge. I'm surprised it compiled. Shouldn't 'ep;' be 'ep:;'? I don't know since it is 43 years since I last wrote a goto statement (excluding for old languages that needed 'ON ERROR GOTO ..', and that was as recent as the late 1990s).

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

                              Quote:

                              I'm surprised it compiled.

                              I strongly doubt it compiles.

                              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
                                Fueled By Decaff
                                wrote on last edited by
                                #18

                                First thoughts: It does not look very performant with 4 nested for loops, but it might be necessary, so need more information before commenting any more. Blink twice thoughts: Nuke that goto. Nuke it from space... This should not compile in C#. Wouldn't that have undefined behaviour in C, as the loop variables would not get initialised? It looks like the code that the goto label refers to is only executed from the goto, so perhaps the goto could be replaced by moving that code into the if statement. Good luck untangling this code.

                                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
                                  DerekT P
                                  wrote on last edited by
                                  #19

                                  Well, it shows a good deal of promise. The author has very rigorously and helpfully identified (via comments) both the end of the method, and the end of the last for loop. Clearly s/he was in a great hurry, or maybe just wilfully lazy, as there is no equivalent commenting on the end of the inner loops. A smack on the wrist needed there, I think. Succinct and meaningful variable names used. i_n_ suggests a background in early BASIC, but that's fine; at least we know which level each variable relates to. There's a flag helpfully named flag, and a condition sensibly named pass. Well thought through. My criticism would be that ep is obviously the "end procedure" goto target, but to perpetuate the camel case standard used in the method name, it should be eP. Indentation is good and consistent. Not sure on the inline if but that may conform to local standards. Otherwise? meh. :|

                                  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
                                    Jacquers
                                    wrote on last edited by
                                    #20

                                    Without context on what it's doing and trying to solve it's a bit difficult to say, but it looks klunky and if I had to review it I'd probably fail it.

                                    1 Reply Last reply
                                    0
                                    • OriginalGriffO OriginalGriff

                                      Apparently so, in C++. In C# (his target language) no - the label is out of scope of the goto. It was apparently written in 2006 ... and in my opinion you shouldn't try to perpetuate code crimes into new languages. To be honest, any coder who thinks nesting loops 5+ deep is a good idea probably needs a lesson in exponential growth ... :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
                                      Fueled By Decaff
                                      wrote on last edited by
                                      #21

                                      Ooh, I thought C++ initialised variables to 0 by default - apparently not. So this is undefined behaviour if any of the loop variables are used after the goto is executed. Fnu

                                      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!

                                        P Offline
                                        P Offline
                                        Pete OHanlon
                                        wrote on last edited by
                                        #22

                                        Without context, it's hard to say what's going on but a thought occurs to me here; if there is no condition before that return statement, each loop level is going to execute only once so the whole logic could be flattened.

                                        Advanced TypeScript Programming Projects

                                        T 1 Reply Last reply
                                        0
                                        • P Pete OHanlon

                                          Without context, it's hard to say what's going on but a thought occurs to me here; if there is no condition before that return statement, each loop level is going to execute only once so the whole logic could be flattened.

                                          Advanced TypeScript Programming Projects

                                          T Offline
                                          T Offline
                                          theoldfool
                                          wrote on last edited by
                                          #23

                                          who gave you permission to post my code on here? I thought the red printing at the top forbid that. edit: oops, sorry meant to reply to that other fellow.

                                          If you can keep your head while those about you are losing theirs, perhaps you don't understand the situation.

                                          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