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. I don't like code reviews

I don't like code reviews

Scheduled Pinned Locked Moved The Lounge
csharpcomperformancetutorialquestion
74 Posts 33 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 Super Lloyd

    ok, sometimes there are very good comments... but every time the reviews are waaaaay too slow. and very often there are comments which are both useless, antagonistic and a big waste of time... for example I don't see the point of long variable name nor do I like them, particularly for a short liner like

    double Value
    {
    get
    {
    var x = Calculation();
    return flag ? x : 2 * x;
    }
    }

    And have to wait a few more hours because I was told 'not to use short variable name'. Unsure I renamed 'x' to 'aNumber', but that irks me... On top of that, that might be just me with my bad memory, but I find long variable name harder to read! :omg: For example a simple expression like a = b + c can confuse me if you write instead myobjectBlu = aCycleValueOrdinal + meteorStrikeOffsetTime. Why they not care about making the code easier to understand?! :(( ok, ok, I need to get over it. just venting here! :laugh: Joke aside, you might like long variable name, but you won't convince me. save everyone's time and let's just agree to disagree. Or disagree to disagree, if you prefer... EDIT Upon reflection, I might be part of a minority of people with reading disability.. :(( When reading long sentence I am skipping words and filling in by guess. Similarly long line of C# requires me multiple reading. And it kind of depends on the overall number of character, not words... So I guess normal people comes with their usually suck it up, I am fine... :sigh:

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

    D Offline
    D Offline
    dandy72
    wrote on last edited by
    #29

    Super Lloyd wrote:

    For example a simple expression like a = b + c can confuse me if you write instead myobjectBlu = aCycleValueOrdinal + meteorStrikeOffsetTime.

    Good example. Without greater context, I can't tell whether "b + c" is the correct thing to do to get "a". Given your second example however, if I know that in order to get myobjectBlu, the equation needs meteorStrikeOffsetTime to be multiplied by a constant before being added to aCycleValueOrdinal, then I can spot this. [Edit] Just being devil's advocate. I have to imagine that someone reviewing this would see the greater context and not just look at the one line.

    1 Reply Last reply
    0
    • J Jorgen Andersson

      Oh, I thought for a short while it was the Schrödinger operator.

      Wrong is evil and must be defeated. - Jeff Ello

      B Offline
      B Offline
      BillWoodruff
      wrote on last edited by
      #30

      Some say the Schrodinger operator is kind of half-assed; I agree with them half the time :wtf:

      «The mind is not a vessel to be filled but a fire to be kindled» Plutarch

      1 Reply Last reply
      0
      • S Super Lloyd

        ok, sometimes there are very good comments... but every time the reviews are waaaaay too slow. and very often there are comments which are both useless, antagonistic and a big waste of time... for example I don't see the point of long variable name nor do I like them, particularly for a short liner like

        double Value
        {
        get
        {
        var x = Calculation();
        return flag ? x : 2 * x;
        }
        }

        And have to wait a few more hours because I was told 'not to use short variable name'. Unsure I renamed 'x' to 'aNumber', but that irks me... On top of that, that might be just me with my bad memory, but I find long variable name harder to read! :omg: For example a simple expression like a = b + c can confuse me if you write instead myobjectBlu = aCycleValueOrdinal + meteorStrikeOffsetTime. Why they not care about making the code easier to understand?! :(( ok, ok, I need to get over it. just venting here! :laugh: Joke aside, you might like long variable name, but you won't convince me. save everyone's time and let's just agree to disagree. Or disagree to disagree, if you prefer... EDIT Upon reflection, I might be part of a minority of people with reading disability.. :(( When reading long sentence I am skipping words and filling in by guess. Similarly long line of C# requires me multiple reading. And it kind of depends on the overall number of character, not words... So I guess normal people comes with their usually suck it up, I am fine... :sigh:

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

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

        Super Lloyd wrote:

        double Value { get { var x = Calculation(); return flag ? x : 2 * x; } }

        Here's my comment - do NOT use var! Using var instead of double forced me to look in a second place in your code to figure out what I was reading. var is an abomination.

        S 1 Reply Last reply
        0
        • O obermd

          Super Lloyd wrote:

          double Value { get { var x = Calculation(); return flag ? x : 2 * x; } }

          Here's my comment - do NOT use var! Using var instead of double forced me to look in a second place in your code to figure out what I was reading. var is an abomination.

          S Offline
          S Offline
          Slacker007
          wrote on last edited by
          #32

          obermd wrote:

          var is an abomination.

          when used improperly. Our shop uses var all the time. var myClass = new MyMostExcellentClassInTheWholeWideWorld(); this is acceptable IMHO and the opinions of hundreds of thousands of developers world wide, if not millions, billions, and trillions. var something1 = something2.GetSomething3(); this is not acceptable. This is a hotly debated topic, I know. To each there own.

          B O 2 Replies Last reply
          0
          • S Super Lloyd

            design by committee... the plight of any big corp! ^_^

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

            B Offline
            B Offline
            BillWoodruff
            wrote on last edited by
            #33

            The code bases of the existing Adobe products I worked on, PhotoShop, Illustrator, were the opposite of design by committee: they grew by accretion. New, "hot," features always took precedence over any systematic re-org. By the way, I have complete respect for the world-class programmers I had the chance to work with, and, I felt continually humble in their presence ... for good reasons: I was a one-trick pony :) When they did the PC version of PhotoShop, they found the Mac code (using the MacApp framework) impossible to port "native," so they re-implemented MacApp on Windows, which took a tremendous effort. [^]

            «The mind is not a vessel to be filled but a fire to be kindled» Plutarch

            1 Reply Last reply
            0
            • S Slacker007

              obermd wrote:

              var is an abomination.

              when used improperly. Our shop uses var all the time. var myClass = new MyMostExcellentClassInTheWholeWideWorld(); this is acceptable IMHO and the opinions of hundreds of thousands of developers world wide, if not millions, billions, and trillions. var something1 = something2.GetSomething3(); this is not acceptable. This is a hotly debated topic, I know. To each there own.

              B Offline
              B Offline
              BillWoodruff
              wrote on last edited by
              #34

              and now, C# 9 brings you "even more naked" instantiation: List xs = new(); List? ys = new();

              «The mind is not a vessel to be filled but a fire to be kindled» Plutarch

              S 1 Reply Last reply
              0
              • S Super Lloyd

                ok, sometimes there are very good comments... but every time the reviews are waaaaay too slow. and very often there are comments which are both useless, antagonistic and a big waste of time... for example I don't see the point of long variable name nor do I like them, particularly for a short liner like

                double Value
                {
                get
                {
                var x = Calculation();
                return flag ? x : 2 * x;
                }
                }

                And have to wait a few more hours because I was told 'not to use short variable name'. Unsure I renamed 'x' to 'aNumber', but that irks me... On top of that, that might be just me with my bad memory, but I find long variable name harder to read! :omg: For example a simple expression like a = b + c can confuse me if you write instead myobjectBlu = aCycleValueOrdinal + meteorStrikeOffsetTime. Why they not care about making the code easier to understand?! :(( ok, ok, I need to get over it. just venting here! :laugh: Joke aside, you might like long variable name, but you won't convince me. save everyone's time and let's just agree to disagree. Or disagree to disagree, if you prefer... EDIT Upon reflection, I might be part of a minority of people with reading disability.. :(( When reading long sentence I am skipping words and filling in by guess. Similarly long line of C# requires me multiple reading. And it kind of depends on the overall number of character, not words... So I guess normal people comes with their usually suck it up, I am fine... :sigh:

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

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

                I agree, the length of a variable name should be proportional to its scope and visibility. The reader should be able to see a one-letter variable and just know that it has only local scope. Any developer who gets confused by such a thing does not belong on my team. The fewer the characters, the easier to read. :-D

                O 1 Reply Last reply
                0
                • B BillWoodruff

                  and now, C# 9 brings you "even more naked" instantiation: List xs = new(); List? ys = new();

                  «The mind is not a vessel to be filled but a fire to be kindled» Plutarch

                  S Offline
                  S Offline
                  Slacker007
                  wrote on last edited by
                  #36

                  BillWoodruff wrote:

                  List<int> xs = new(); List<int>? ys = new();

                  stuff like this is just retarded. it is really for the people who hate "var". These people want to see the instantiation on the left, rather than the right. It makes me laugh and cry all the way home.

                  O 1 Reply Last reply
                  0
                  • S Super Lloyd

                    ok, sometimes there are very good comments... but every time the reviews are waaaaay too slow. and very often there are comments which are both useless, antagonistic and a big waste of time... for example I don't see the point of long variable name nor do I like them, particularly for a short liner like

                    double Value
                    {
                    get
                    {
                    var x = Calculation();
                    return flag ? x : 2 * x;
                    }
                    }

                    And have to wait a few more hours because I was told 'not to use short variable name'. Unsure I renamed 'x' to 'aNumber', but that irks me... On top of that, that might be just me with my bad memory, but I find long variable name harder to read! :omg: For example a simple expression like a = b + c can confuse me if you write instead myobjectBlu = aCycleValueOrdinal + meteorStrikeOffsetTime. Why they not care about making the code easier to understand?! :(( ok, ok, I need to get over it. just venting here! :laugh: Joke aside, you might like long variable name, but you won't convince me. save everyone's time and let's just agree to disagree. Or disagree to disagree, if you prefer... EDIT Upon reflection, I might be part of a minority of people with reading disability.. :(( When reading long sentence I am skipping words and filling in by guess. Similarly long line of C# requires me multiple reading. And it kind of depends on the overall number of character, not words... So I guess normal people comes with their usually suck it up, I am fine... :sigh:

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

                    K Offline
                    K Offline
                    kmoorevs
                    wrote on last edited by
                    #37

                    In 23 years, I've never had my code reviewed. I feel lucky! :)

                    "Go forth into the source" - Neal Morse "Hope is contagious"

                    B 1 Reply Last reply
                    0
                    • S Slacker007

                      obermd wrote:

                      var is an abomination.

                      when used improperly. Our shop uses var all the time. var myClass = new MyMostExcellentClassInTheWholeWideWorld(); this is acceptable IMHO and the opinions of hundreds of thousands of developers world wide, if not millions, billions, and trillions. var something1 = something2.GetSomething3(); this is not acceptable. This is a hotly debated topic, I know. To each there own.

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

                      This example works because the class name is in the same line of code.

                      1 Reply Last reply
                      0
                      • S Slacker007

                        BillWoodruff wrote:

                        List<int> xs = new(); List<int>? ys = new();

                        stuff like this is just retarded. it is really for the people who hate "var". These people want to see the instantiation on the left, rather than the right. It makes me laugh and cry all the way home.

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

                        This works as well because it's obvious from the single line of code the type of the variable.

                        1 Reply Last reply
                        0
                        • P PIEBALDconsult

                          I agree, the length of a variable name should be proportional to its scope and visibility. The reader should be able to see a one-letter variable and just know that it has only local scope. Any developer who gets confused by such a thing does not belong on my team. The fewer the characters, the easier to read. :-D

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

                          Thinking about my own code I realize I do this as well. Descriptive names for globals or widely used variables and functions and short names for local use only.

                          1 Reply Last reply
                          0
                          • S Super Lloyd

                            ok, sometimes there are very good comments... but every time the reviews are waaaaay too slow. and very often there are comments which are both useless, antagonistic and a big waste of time... for example I don't see the point of long variable name nor do I like them, particularly for a short liner like

                            double Value
                            {
                            get
                            {
                            var x = Calculation();
                            return flag ? x : 2 * x;
                            }
                            }

                            And have to wait a few more hours because I was told 'not to use short variable name'. Unsure I renamed 'x' to 'aNumber', but that irks me... On top of that, that might be just me with my bad memory, but I find long variable name harder to read! :omg: For example a simple expression like a = b + c can confuse me if you write instead myobjectBlu = aCycleValueOrdinal + meteorStrikeOffsetTime. Why they not care about making the code easier to understand?! :(( ok, ok, I need to get over it. just venting here! :laugh: Joke aside, you might like long variable name, but you won't convince me. save everyone's time and let's just agree to disagree. Or disagree to disagree, if you prefer... EDIT Upon reflection, I might be part of a minority of people with reading disability.. :(( When reading long sentence I am skipping words and filling in by guess. Similarly long line of C# requires me multiple reading. And it kind of depends on the overall number of character, not words... So I guess normal people comes with their usually suck it up, I am fine... :sigh:

                            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
                            #41

                            I use "i" in for loops. That's it. Edit: Also using "x and y" currently: for position coordinates.

                            It was only in wine that he laid down no limit for himself, but he did not allow himself to be confused by it. ― Confucian Analects: Rules of Confucius about his food

                            1 Reply Last reply
                            0
                            • S Super Lloyd

                              so we disagree to disagree then? I find the later no easier.. In fact some interesting brain chemistry must be at work here... I was reflecting how physicist (that's my background), prefer short name too, i.e. it'e E=mc^2, not Energy = Mass * SpeedOfLight^2, to vindicate me... Anyway, regardless, it's more interesting to consider what psychological factor lead from one to another. I know that for me, bad work memory favor short variable names. Long variable names are just too hard, I have to read the statement 2 or 3 times to get it. 1 or 2 time to get all the variables involved, and one more time to get the computation. I can get all that in one go/read with shorter text - i.e. short variable names and simple math. Maybe I have some sort of dyslexia or something, I tend to not read big wall of text very accurately. Not just in code but also in plain English... Hence for me shorter variable name increasing my accuracy / understanding... :sigh:

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

                              D Offline
                              D Offline
                              David ONeil
                              wrote on last edited by
                              #42

                              Super Lloyd wrote:

                              so we disagree to disagree then?

                              No. If I was the boss and an employee used Calculate as a function name I'd shitcan him or her, unless it was obviously a joke and they fixed it as soon as the laugh was had. I would not let a codebase I am responsible for be polluted by such meaningless names that all coders from there on out are going to have to spend precious time figuring out the intent of even something this simple. Among the oldest of business mantras is "Time is Money". And coding like this wastes everyone else's time. Making a lone coder happy, even if they are good, isn't worth it in cases like this.

                              The Science of King David's Court | Object Oriented Programming with C++

                              1 Reply Last reply
                              0
                              • S Super Lloyd

                                ok, sometimes there are very good comments... but every time the reviews are waaaaay too slow. and very often there are comments which are both useless, antagonistic and a big waste of time... for example I don't see the point of long variable name nor do I like them, particularly for a short liner like

                                double Value
                                {
                                get
                                {
                                var x = Calculation();
                                return flag ? x : 2 * x;
                                }
                                }

                                And have to wait a few more hours because I was told 'not to use short variable name'. Unsure I renamed 'x' to 'aNumber', but that irks me... On top of that, that might be just me with my bad memory, but I find long variable name harder to read! :omg: For example a simple expression like a = b + c can confuse me if you write instead myobjectBlu = aCycleValueOrdinal + meteorStrikeOffsetTime. Why they not care about making the code easier to understand?! :(( ok, ok, I need to get over it. just venting here! :laugh: Joke aside, you might like long variable name, but you won't convince me. save everyone's time and let's just agree to disagree. Or disagree to disagree, if you prefer... EDIT Upon reflection, I might be part of a minority of people with reading disability.. :(( When reading long sentence I am skipping words and filling in by guess. Similarly long line of C# requires me multiple reading. And it kind of depends on the overall number of character, not words... So I guess normal people comes with their usually suck it up, I am fine... :sigh:

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

                                G Offline
                                G Offline
                                Gary R Wheeler
                                wrote on last edited by
                                #43

                                I use descriptive names, which tend to be long. Short names in my code depend upon context. Here's an example:

                                for (int JMi = 0; JMi < JettingModules.Length; JMi++)
                                {
                                JettingModules[JMi].DoStuff(...);
                                }

                                I will occasionally use short names synonymously to long names in order to make a complex expression simpler to understand or read:

                                bool ready = StitchCalibration.Context.Ready(...);
                                bool online = Framework.Online && DFE.Service.Connected;
                                int retry = 0;
                                while (ready && online && (retry < 3))
                                {
                                //...
                                }

                                My names are chosen to tie related concepts together and for clarity of expression.

                                Software Zen: delete this;

                                B 1 Reply Last reply
                                0
                                • R Rage

                                  Super Lloyd wrote:

                                  so we disagree to disagree then?

                                  You agree to disagree. If you disagree to disagree, then you agree.

                                  Do not escape reality : improve reality !

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

                                  When I came up with that sentence, I suddenly get this meaning... Disagree to disagree is when you keep arguing with the hope I'll eventually agree... Agree to disagree is when we both realize we ain't gonna agree...

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

                                  1 Reply Last reply
                                  0
                                  • G Gary R Wheeler

                                    I use descriptive names, which tend to be long. Short names in my code depend upon context. Here's an example:

                                    for (int JMi = 0; JMi < JettingModules.Length; JMi++)
                                    {
                                    JettingModules[JMi].DoStuff(...);
                                    }

                                    I will occasionally use short names synonymously to long names in order to make a complex expression simpler to understand or read:

                                    bool ready = StitchCalibration.Context.Ready(...);
                                    bool online = Framework.Online && DFE.Service.Connected;
                                    int retry = 0;
                                    while (ready && online && (retry < 3))
                                    {
                                    //...
                                    }

                                    My names are chosen to tie related concepts together and for clarity of expression.

                                    Software Zen: delete this;

                                    B Offline
                                    B Offline
                                    BillWoodruff
                                    wrote on last edited by
                                    #45

                                    "depends on context" Amen !

                                    «The mind is not a vessel to be filled but a fire to be kindled» Plutarch

                                    1 Reply Last reply
                                    0
                                    • K kmoorevs

                                      In 23 years, I've never had my code reviewed. I feel lucky! :)

                                      "Go forth into the source" - Neal Morse "Hope is contagious"

                                      B Offline
                                      B Offline
                                      BernardIE5317
                                      wrote on last edited by
                                      #46

                                      Greetings I have never had my code reviewed either but wouldn't mind if it were I might learn something or perhaps teach the reviewer a thing or two - Cheerio PS As for coding styles I also prefer long descriptive names for identifiers which set the context and short for those in context Also I have always used different naming conventions to indicate scope i.e. local class global This varies though I always utilize snake for local and am tending to camel for class and 'g' prefix for the rare global "I once put instant coffee into the microwave and went back in time." - Steven Wright "Shut up and calculate" - apparently N. David Mermin possibly Richard Feynman

                                      1 Reply Last reply
                                      0
                                      • S Super Lloyd

                                        ok, sometimes there are very good comments... but every time the reviews are waaaaay too slow. and very often there are comments which are both useless, antagonistic and a big waste of time... for example I don't see the point of long variable name nor do I like them, particularly for a short liner like

                                        double Value
                                        {
                                        get
                                        {
                                        var x = Calculation();
                                        return flag ? x : 2 * x;
                                        }
                                        }

                                        And have to wait a few more hours because I was told 'not to use short variable name'. Unsure I renamed 'x' to 'aNumber', but that irks me... On top of that, that might be just me with my bad memory, but I find long variable name harder to read! :omg: For example a simple expression like a = b + c can confuse me if you write instead myobjectBlu = aCycleValueOrdinal + meteorStrikeOffsetTime. Why they not care about making the code easier to understand?! :(( ok, ok, I need to get over it. just venting here! :laugh: Joke aside, you might like long variable name, but you won't convince me. save everyone's time and let's just agree to disagree. Or disagree to disagree, if you prefer... EDIT Upon reflection, I might be part of a minority of people with reading disability.. :(( When reading long sentence I am skipping words and filling in by guess. Similarly long line of C# requires me multiple reading. And it kind of depends on the overall number of character, not words... So I guess normal people comes with their usually suck it up, I am fine... :sigh:

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

                                        B Offline
                                        B Offline
                                        BernardIE5317
                                        wrote on last edited by
                                        #47

                                        Greetings My code has never been reviewed but am curious if it were I might learn something or teach the reviewer a thing or two Wouldn't mind reviewing others' for the same reason- Cheerio PS As for naming It seems obvious names should be as long as necessary to indicate intent when they set context Short is fine once context is set Also the name can indicate the type and value range of the identifier e.g. "calculateWidth_ofImpendingMeteorStroke" will never return a negative value Further have some sympathy for the poor chap who will maintain the code or for yourself many months or years hence Further I like to utilize different naming conventions to indicate the scope of the identifier I always utilize snake for local and now tend to camel for class and prefix with a 'g' for the rare global "I once put instant coffee into the microwave and went back in time." - Steven Wright "Shut up and calculate" - apparently N. David Mermin possibly Richard Feynman

                                        S 1 Reply Last reply
                                        0
                                        • B BernardIE5317

                                          Greetings My code has never been reviewed but am curious if it were I might learn something or teach the reviewer a thing or two Wouldn't mind reviewing others' for the same reason- Cheerio PS As for naming It seems obvious names should be as long as necessary to indicate intent when they set context Short is fine once context is set Also the name can indicate the type and value range of the identifier e.g. "calculateWidth_ofImpendingMeteorStroke" will never return a negative value Further have some sympathy for the poor chap who will maintain the code or for yourself many months or years hence Further I like to utilize different naming conventions to indicate the scope of the identifier I always utilize snake for local and now tend to camel for class and prefix with a 'g' for the rare global "I once put instant coffee into the microwave and went back in time." - Steven Wright "Shut up and calculate" - apparently N. David Mermin possibly Richard Feynman

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

                                          If you use Git you are likely, but not necessarily, to have review. (not using Git here thought) At any rate, certainly, every now and then there is very good feedback in review. However every single time it waste a lot of time.

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

                                          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