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. code review

code review

Scheduled Pinned Locked Moved The Lounge
csharpcomquestioncode-review
31 Posts 14 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.
  • S Offline
    S Offline
    Super Lloyd
    wrote on last edited by
    #1

    I often feel like code review are mostly filled with unneeded comment for the sake of commenting or to take some sort of "ownership" that doesn't do anything special beside burdening the reviewee with a special cosmetic change udpate. Anyway, someone was updating my code and I was feeling revengeful so I also asserted my opinion on purely cosmetic and totally irrelevant code! ;P Although, come to think of it, the guy was doing unnecessary work in Dispose() because "that's what is done everywhere", even though it's not needed and closing documents take godamn too long (due to all those unnecessary piece of code running in all those Dispose() methods), mmmrrppphhh... I stick to my gun!

    A new .NET Serializer All in one Menu-Ribbon Bar Taking over the world since 1371!

    T M L U D 8 Replies Last reply
    0
    • S Super Lloyd

      I often feel like code review are mostly filled with unneeded comment for the sake of commenting or to take some sort of "ownership" that doesn't do anything special beside burdening the reviewee with a special cosmetic change udpate. Anyway, someone was updating my code and I was feeling revengeful so I also asserted my opinion on purely cosmetic and totally irrelevant code! ;P Although, come to think of it, the guy was doing unnecessary work in Dispose() because "that's what is done everywhere", even though it's not needed and closing documents take godamn too long (due to all those unnecessary piece of code running in all those Dispose() methods), mmmrrppphhh... I stick to my gun!

      A new .NET Serializer All in one Menu-Ribbon Bar Taking over the world since 1371!

      T Offline
      T Offline
      trønderen
      wrote on last edited by
      #2

      Super Lloyd wrote:

      Anyway, someone was updating my code

      One of my earlier co-workers discovered that my code contained a '#define ever ;;' so that I could write an infinite loop as 'for (ever) { ...'. A few years earlier, I was working in a CHILL environment; in CHILL 'DO FOR EVER' is a part of the base language. This construction made my coworker so upset that he not only changed it where he had discovered it; he searched through the entire code base of the company for other uses, and found a good handful; I had been programming with 'for (ever) {' for a couple of years by then (in embedded software, infinite loops are commonplace), changing every one of them to 'while (0) {', adding an angry commit comment that requested everybody to refrain from making such 'jokes' in our program code - we are a serious company! Then he brought it up at the scrum, to make sure that everybody would know that in our company, we do not code that way. I asked if we could make it slightly less cryptic by using 'while (true) {', but this fellow would not under any circumstances accept that. According to him, there is one, single way of coding an infinite loop, that is immediately recognized by every programmer in the world, and that is 'while (0)'. So he would not tolerate anything else. He certainly had no formal authority, completing his degree about three years earlier, having worked in the company for half a year, I was 25 years his senior. I didn't have to accept his dictate. But he had an unbearable arrogance and self confidence that I didn't care to fight against, so I just nodded "OK!". At least that episode gave me a story to tell - this is certainly not the first time :-)

      S L G M O 8 Replies Last reply
      0
      • T trønderen

        Super Lloyd wrote:

        Anyway, someone was updating my code

        One of my earlier co-workers discovered that my code contained a '#define ever ;;' so that I could write an infinite loop as 'for (ever) { ...'. A few years earlier, I was working in a CHILL environment; in CHILL 'DO FOR EVER' is a part of the base language. This construction made my coworker so upset that he not only changed it where he had discovered it; he searched through the entire code base of the company for other uses, and found a good handful; I had been programming with 'for (ever) {' for a couple of years by then (in embedded software, infinite loops are commonplace), changing every one of them to 'while (0) {', adding an angry commit comment that requested everybody to refrain from making such 'jokes' in our program code - we are a serious company! Then he brought it up at the scrum, to make sure that everybody would know that in our company, we do not code that way. I asked if we could make it slightly less cryptic by using 'while (true) {', but this fellow would not under any circumstances accept that. According to him, there is one, single way of coding an infinite loop, that is immediately recognized by every programmer in the world, and that is 'while (0)'. So he would not tolerate anything else. He certainly had no formal authority, completing his degree about three years earlier, having worked in the company for half a year, I was 25 years his senior. I didn't have to accept his dictate. But he had an unbearable arrogance and self confidence that I didn't care to fight against, so I just nodded "OK!". At least that episode gave me a story to tell - this is certainly not the first time :-)

        S Offline
        S Offline
        Super Lloyd
        wrote on last edited by
        #3

        Mmmh.... shouldn't it be while (1)? Now I am worried your colleague might have broken lots of stuff... Fun story though! I quite like the for (ever) :) :rolleyes:

        A new .NET Serializer All in one Menu-Ribbon Bar Taking over the world since 1371!

        J T 2 Replies Last reply
        0
        • S Super Lloyd

          Mmmh.... shouldn't it be while (1)? Now I am worried your colleague might have broken lots of stuff... Fun story though! I quite like the for (ever) :) :rolleyes:

          A new .NET Serializer All in one Menu-Ribbon Bar Taking over the world since 1371!

          J Offline
          J Offline
          jmaida
          wrote on last edited by
          #4

          for the spiritualists: Do while( 1 ) { I = Live(); }; for the materialists: Do while( 1 ) { I = Make_Money(); }; P.S. No offense intended just being a silly programmer about forever.

          "A little time, a little trouble, your better day" Badfinger

          S D 2 Replies Last reply
          0
          • J jmaida

            for the spiritualists: Do while( 1 ) { I = Live(); }; for the materialists: Do while( 1 ) { I = Make_Money(); }; P.S. No offense intended just being a silly programmer about forever.

            "A little time, a little trouble, your better day" Badfinger

            S Offline
            S Offline
            Super Lloyd
            wrote on last edited by
            #5

            Do while (ever) { I = TryToLive() && MakeMoney(); }

            A new .NET Serializer All in one Menu-Ribbon Bar Taking over the world since 1371!

            1 Reply Last reply
            0
            • S Super Lloyd

              Mmmh.... shouldn't it be while (1)? Now I am worried your colleague might have broken lots of stuff... Fun story though! I quite like the for (ever) :) :rolleyes:

              A new .NET Serializer All in one Menu-Ribbon Bar Taking over the world since 1371!

              T Offline
              T Offline
              trønderen
              wrote on last edited by
              #6

              Super Lloyd wrote:

              Mmmh.... shouldn't it be while (1)?

              Of course! Sorry about that typo. I prefer 'for (ever)' :-)

              1 Reply Last reply
              0
              • S Super Lloyd

                I often feel like code review are mostly filled with unneeded comment for the sake of commenting or to take some sort of "ownership" that doesn't do anything special beside burdening the reviewee with a special cosmetic change udpate. Anyway, someone was updating my code and I was feeling revengeful so I also asserted my opinion on purely cosmetic and totally irrelevant code! ;P Although, come to think of it, the guy was doing unnecessary work in Dispose() because "that's what is done everywhere", even though it's not needed and closing documents take godamn too long (due to all those unnecessary piece of code running in all those Dispose() methods), mmmrrppphhh... I stick to my gun!

                A new .NET Serializer All in one Menu-Ribbon Bar Taking over the world since 1371!

                M Offline
                M Offline
                megaadam
                wrote on last edited by
                #7

                You speak of of your conclusion as if it was universal. Maybe your workplace culture leads to poor code reviews. That does not mean that code reviews cannot be done in a productive way. Cheerz.

                "If we don't change direction, we'll end up where we're going"

                S 1 Reply Last reply
                0
                • J jmaida

                  for the spiritualists: Do while( 1 ) { I = Live(); }; for the materialists: Do while( 1 ) { I = Make_Money(); }; P.S. No offense intended just being a silly programmer about forever.

                  "A little time, a little trouble, your better day" Badfinger

                  D Offline
                  D Offline
                  Daniel Pfeffer
                  wrote on last edited by
                  #8

                  try
                  {
                  Be_Born();
                  while (1)
                  {
                  try
                  {
                  Live();
                  Make_Money();
                  }
                  catch (FinancialException const& e)
                  {
                  // handle financial exception
                  }
                  catch (MedicalException const& e)
                  {
                  // handle medical exception
                  }
                  }

                  }
                  catch (DeathException const& e)
                  {
                  Die();
                  }

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

                  J 2 Replies Last reply
                  0
                  • T trønderen

                    Super Lloyd wrote:

                    Anyway, someone was updating my code

                    One of my earlier co-workers discovered that my code contained a '#define ever ;;' so that I could write an infinite loop as 'for (ever) { ...'. A few years earlier, I was working in a CHILL environment; in CHILL 'DO FOR EVER' is a part of the base language. This construction made my coworker so upset that he not only changed it where he had discovered it; he searched through the entire code base of the company for other uses, and found a good handful; I had been programming with 'for (ever) {' for a couple of years by then (in embedded software, infinite loops are commonplace), changing every one of them to 'while (0) {', adding an angry commit comment that requested everybody to refrain from making such 'jokes' in our program code - we are a serious company! Then he brought it up at the scrum, to make sure that everybody would know that in our company, we do not code that way. I asked if we could make it slightly less cryptic by using 'while (true) {', but this fellow would not under any circumstances accept that. According to him, there is one, single way of coding an infinite loop, that is immediately recognized by every programmer in the world, and that is 'while (0)'. So he would not tolerate anything else. He certainly had no formal authority, completing his degree about three years earlier, having worked in the company for half a year, I was 25 years his senior. I didn't have to accept his dictate. But he had an unbearable arrogance and self confidence that I didn't care to fight against, so I just nodded "OK!". At least that episode gave me a story to tell - this is certainly not the first time :-)

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

                    I faced a similar situation years ago, and met a similar response. Largely because the people "across the pond" could not get their heads around how time zones work, and the 24-hour clock.

                    1 Reply Last reply
                    0
                    • S Super Lloyd

                      I often feel like code review are mostly filled with unneeded comment for the sake of commenting or to take some sort of "ownership" that doesn't do anything special beside burdening the reviewee with a special cosmetic change udpate. Anyway, someone was updating my code and I was feeling revengeful so I also asserted my opinion on purely cosmetic and totally irrelevant code! ;P Although, come to think of it, the guy was doing unnecessary work in Dispose() because "that's what is done everywhere", even though it's not needed and closing documents take godamn too long (due to all those unnecessary piece of code running in all those Dispose() methods), mmmrrppphhh... I stick to my gun!

                      A new .NET Serializer All in one Menu-Ribbon Bar Taking over the world since 1371!

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

                      I review books. From Manning. I never had to ask them to change code. Ever. If you want to make a proposition for change, you ask yourself what it is worth. Does your idea add value? If not, then SHUT THE FUCK UP.

                      Bastard Programmer from Hell :suss: "If you just follow the bacon Eddy, wherever it leads you, then you won't have to think about politics." -- Some Bell.

                      1 Reply Last reply
                      0
                      • T trønderen

                        Super Lloyd wrote:

                        Anyway, someone was updating my code

                        One of my earlier co-workers discovered that my code contained a '#define ever ;;' so that I could write an infinite loop as 'for (ever) { ...'. A few years earlier, I was working in a CHILL environment; in CHILL 'DO FOR EVER' is a part of the base language. This construction made my coworker so upset that he not only changed it where he had discovered it; he searched through the entire code base of the company for other uses, and found a good handful; I had been programming with 'for (ever) {' for a couple of years by then (in embedded software, infinite loops are commonplace), changing every one of them to 'while (0) {', adding an angry commit comment that requested everybody to refrain from making such 'jokes' in our program code - we are a serious company! Then he brought it up at the scrum, to make sure that everybody would know that in our company, we do not code that way. I asked if we could make it slightly less cryptic by using 'while (true) {', but this fellow would not under any circumstances accept that. According to him, there is one, single way of coding an infinite loop, that is immediately recognized by every programmer in the world, and that is 'while (0)'. So he would not tolerate anything else. He certainly had no formal authority, completing his degree about three years earlier, having worked in the company for half a year, I was 25 years his senior. I didn't have to accept his dictate. But he had an unbearable arrogance and self confidence that I didn't care to fight against, so I just nodded "OK!". At least that episode gave me a story to tell - this is certainly not the first time :-)

                        G Offline
                        G Offline
                        GuyThiebaut
                        wrote on last edited by
                        #11

                        trønderen wrote:

                        'while (0)'

                        What on earth does that do? :sigh: :laugh:

                        “That which can be asserted without evidence, can be dismissed without evidence.”

                        ― Christopher Hitchens

                        S T 2 Replies Last reply
                        0
                        • G GuyThiebaut

                          trønderen wrote:

                          'while (0)'

                          What on earth does that do? :sigh: :laugh:

                          “That which can be asserted without evidence, can be dismissed without evidence.”

                          ― Christopher Hitchens

                          S Offline
                          S Offline
                          Super Lloyd
                          wrote on last edited by
                          #12

                          while (never)

                          A new .NET Serializer All in one Menu-Ribbon Bar Taking over the world since 1371!

                          1 Reply Last reply
                          0
                          • G GuyThiebaut

                            trønderen wrote:

                            'while (0)'

                            What on earth does that do? :sigh: :laugh:

                            “That which can be asserted without evidence, can be dismissed without evidence.”

                            ― Christopher Hitchens

                            T Offline
                            T Offline
                            trønderen
                            wrote on last edited by
                            #13

                            Sorry about the typo - if you had read the response from Super Lloyd and my reply to him, you would have known. I never use 'while (1)' myself - I think of it as a deliberate case of explicit code obfuscation. Another common practice in the same company was to enclose code under development in 'if (0) {...', to have it syntax checked even though it was not ready for execution yet. I guess that is what caused me to write 'while (0)' rather than 'while (1)'.

                            1 Reply Last reply
                            0
                            • T trønderen

                              Super Lloyd wrote:

                              Anyway, someone was updating my code

                              One of my earlier co-workers discovered that my code contained a '#define ever ;;' so that I could write an infinite loop as 'for (ever) { ...'. A few years earlier, I was working in a CHILL environment; in CHILL 'DO FOR EVER' is a part of the base language. This construction made my coworker so upset that he not only changed it where he had discovered it; he searched through the entire code base of the company for other uses, and found a good handful; I had been programming with 'for (ever) {' for a couple of years by then (in embedded software, infinite loops are commonplace), changing every one of them to 'while (0) {', adding an angry commit comment that requested everybody to refrain from making such 'jokes' in our program code - we are a serious company! Then he brought it up at the scrum, to make sure that everybody would know that in our company, we do not code that way. I asked if we could make it slightly less cryptic by using 'while (true) {', but this fellow would not under any circumstances accept that. According to him, there is one, single way of coding an infinite loop, that is immediately recognized by every programmer in the world, and that is 'while (0)'. So he would not tolerate anything else. He certainly had no formal authority, completing his degree about three years earlier, having worked in the company for half a year, I was 25 years his senior. I didn't have to accept his dictate. But he had an unbearable arrogance and self confidence that I didn't care to fight against, so I just nodded "OK!". At least that episode gave me a story to tell - this is certainly not the first time :-)

                              M Offline
                              M Offline
                              Mircea Neacsu
                              wrote on last edited by
                              #14

                              Compared with while(1), (8 keystrokes) for(ever) has 9 keystrokes and is a distant third place from for(;;) at only 7 keystrokes :laugh: Sorry I'm lazy; I was born that way.

                              Mircea

                              T 1 Reply Last reply
                              0
                              • T trønderen

                                Super Lloyd wrote:

                                Anyway, someone was updating my code

                                One of my earlier co-workers discovered that my code contained a '#define ever ;;' so that I could write an infinite loop as 'for (ever) { ...'. A few years earlier, I was working in a CHILL environment; in CHILL 'DO FOR EVER' is a part of the base language. This construction made my coworker so upset that he not only changed it where he had discovered it; he searched through the entire code base of the company for other uses, and found a good handful; I had been programming with 'for (ever) {' for a couple of years by then (in embedded software, infinite loops are commonplace), changing every one of them to 'while (0) {', adding an angry commit comment that requested everybody to refrain from making such 'jokes' in our program code - we are a serious company! Then he brought it up at the scrum, to make sure that everybody would know that in our company, we do not code that way. I asked if we could make it slightly less cryptic by using 'while (true) {', but this fellow would not under any circumstances accept that. According to him, there is one, single way of coding an infinite loop, that is immediately recognized by every programmer in the world, and that is 'while (0)'. So he would not tolerate anything else. He certainly had no formal authority, completing his degree about three years earlier, having worked in the company for half a year, I was 25 years his senior. I didn't have to accept his dictate. But he had an unbearable arrogance and self confidence that I didn't care to fight against, so I just nodded "OK!". At least that episode gave me a story to tell - this is certainly not the first time :-)

                                O Offline
                                O Offline
                                obermd
                                wrote on last edited by
                                #15

                                I once wrote a program with a variable int hell_freezes_over = 0; The loop control was do ... until hell_freezes_over The languange didn't support infinite loops.

                                T 1 Reply Last reply
                                0
                                • T trønderen

                                  Super Lloyd wrote:

                                  Anyway, someone was updating my code

                                  One of my earlier co-workers discovered that my code contained a '#define ever ;;' so that I could write an infinite loop as 'for (ever) { ...'. A few years earlier, I was working in a CHILL environment; in CHILL 'DO FOR EVER' is a part of the base language. This construction made my coworker so upset that he not only changed it where he had discovered it; he searched through the entire code base of the company for other uses, and found a good handful; I had been programming with 'for (ever) {' for a couple of years by then (in embedded software, infinite loops are commonplace), changing every one of them to 'while (0) {', adding an angry commit comment that requested everybody to refrain from making such 'jokes' in our program code - we are a serious company! Then he brought it up at the scrum, to make sure that everybody would know that in our company, we do not code that way. I asked if we could make it slightly less cryptic by using 'while (true) {', but this fellow would not under any circumstances accept that. According to him, there is one, single way of coding an infinite loop, that is immediately recognized by every programmer in the world, and that is 'while (0)'. So he would not tolerate anything else. He certainly had no formal authority, completing his degree about three years earlier, having worked in the company for half a year, I was 25 years his senior. I didn't have to accept his dictate. But he had an unbearable arrogance and self confidence that I didn't care to fight against, so I just nodded "OK!". At least that episode gave me a story to tell - this is certainly not the first time :-)

                                  D Offline
                                  D Offline
                                  Daniel Pfeffer
                                  wrote on last edited by
                                  #16

                                  And what is wrong with the idiom:

                                  for (;;)
                                  {
                                  // do forever
                                  }

                                  Which has no spurious conditions? Inquiring minds wish to know!

                                  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
                                  • S Super Lloyd

                                    I often feel like code review are mostly filled with unneeded comment for the sake of commenting or to take some sort of "ownership" that doesn't do anything special beside burdening the reviewee with a special cosmetic change udpate. Anyway, someone was updating my code and I was feeling revengeful so I also asserted my opinion on purely cosmetic and totally irrelevant code! ;P Although, come to think of it, the guy was doing unnecessary work in Dispose() because "that's what is done everywhere", even though it's not needed and closing documents take godamn too long (due to all those unnecessary piece of code running in all those Dispose() methods), mmmrrppphhh... I stick to my gun!

                                    A new .NET Serializer All in one Menu-Ribbon Bar Taking over the world since 1371!

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

                                    In one place, we wrote pseudo-code before coding. We reviewed the pseudo code. That's it. (i.e. the programmer / analyst gets it).

                                    "Before entering on an understanding, I have meditated for a long time, and have foreseen what might happen. It is not genius which reveals to me suddenly, secretly, what I have to say or to do in a circumstance unexpected by other people; it is reflection, it is meditation." - Napoleon I

                                    1 Reply Last reply
                                    0
                                    • M megaadam

                                      You speak of of your conclusion as if it was universal. Maybe your workplace culture leads to poor code reviews. That does not mean that code reviews cannot be done in a productive way. Cheerz.

                                      "If we don't change direction, we'll end up where we're going"

                                      S Offline
                                      S Offline
                                      Super Lloyd
                                      wrote on last edited by
                                      #18

                                      well.. lucky you. the larger the team the higher the chance to have that coworker (those coworkers?) who wants to micromanage reviews

                                      A new .NET Serializer All in one Menu-Ribbon Bar Taking over the world since 1371!

                                      M 1 Reply Last reply
                                      0
                                      • D Daniel Pfeffer

                                        try
                                        {
                                        Be_Born();
                                        while (1)
                                        {
                                        try
                                        {
                                        Live();
                                        Make_Money();
                                        }
                                        catch (FinancialException const& e)
                                        {
                                        // handle financial exception
                                        }
                                        catch (MedicalException const& e)
                                        {
                                        // handle medical exception
                                        }
                                        }

                                        }
                                        catch (DeathException const& e)
                                        {
                                        Die();
                                        }

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

                                        J Offline
                                        J Offline
                                        jmaida
                                        wrote on last edited by
                                        #19

                                        makes sense! :)

                                        "A little time, a little trouble, your better day" Badfinger

                                        1 Reply Last reply
                                        0
                                        • M Mircea Neacsu

                                          Compared with while(1), (8 keystrokes) for(ever) has 9 keystrokes and is a distant third place from for(;;) at only 7 keystrokes :laugh: Sorry I'm lazy; I was born that way.

                                          Mircea

                                          T Offline
                                          T Offline
                                          trønderen
                                          wrote on last edited by
                                          #20

                                          APL is the language for you!

                                          M 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