I don't like code reviews
-
Code should be easy to skim for the easy bits, so using descriptive variables is helpful, as is insertion of concise comments, but descriptive needn't be long. Also, as long as you keep in mind the scope of each variable and are doing any trash collection that is needed by your language choice, then short names are fine, especially as iterators. Just don't be lazy and leave a variable living longer than it is needed.
see.. you obviously didn't read what I said. I said it is much harder for me to read those long names. I am cool though, however strange, I believe you when you said you find long names easier... I just beg, don't make me suffer for your personal convenience...
A new .NET Serializer All in one Menu-Ribbon Bar Taking over the world since 1371!
-
I have to agree with David O'Neal - short names are a bad idea. There isn't just the "Understandability" factor, though that is very significant - especially when you look at the code for the first time, or after a long break from it. More significantly one character names are more prone to error, because it's very easy to hit the wrong key and get a valid variable name. While Intellisense can make that happen with more descriptive names as well, it's a lot more obvious that it's wrong. To use your example:
Quote:
it'e E=mc^2, not Energy = Mass * SpeedOfLight^2, to vindicate me...
E=nc^2
Looks vaguely right and it's easy to miss the mistake when skimming, but the longer form:
Energy = numberOfCatsInTheBox * SpeedOfLight^2
Is immediately wrong. And ... if you are anything like me you read what you meant to write, rather than what you did type. So short names make life harder for you, me, and everyone else who has to work with the code - so yes, code reviews should pick them up! I'm not saying that all short names are bad, it's pointless to use a long name here:
for (int i = 0; i < rooms.Count; i++)
{
rooms[i] = new Room();
}But in general, they are a PITA!
"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!
for (int room = 0, roomCount = rooms.Count; room < roomCount; room++)
{
rooms[room] = new Room();
}Il just refuse anything else than something like this, my point of view
-
see.. you obviously didn't read what I said. I said it is much harder for me to read those long names. I am cool though, however strange, I believe you when you said you find long names easier... I just beg, don't make me suffer for your personal convenience...
A new .NET Serializer All in one Menu-Ribbon Bar Taking over the world since 1371!
I did say that descriptive names need not be long, and that iterators need not even be descriptive. Clarity is the goal, so using a name like double_me for a flag might be all that's needed, leaving your input variable as x. Did I say that long names are easier? I don't remember saying that.
-
I have to agree with David O'Neal - short names are a bad idea. There isn't just the "Understandability" factor, though that is very significant - especially when you look at the code for the first time, or after a long break from it. More significantly one character names are more prone to error, because it's very easy to hit the wrong key and get a valid variable name. While Intellisense can make that happen with more descriptive names as well, it's a lot more obvious that it's wrong. To use your example:
Quote:
it'e E=mc^2, not Energy = Mass * SpeedOfLight^2, to vindicate me...
E=nc^2
Looks vaguely right and it's easy to miss the mistake when skimming, but the longer form:
Energy = numberOfCatsInTheBox * SpeedOfLight^2
Is immediately wrong. And ... if you are anything like me you read what you meant to write, rather than what you did type. So short names make life harder for you, me, and everyone else who has to work with the code - so yes, code reviews should pick them up! I'm not saying that all short names are bad, it's pointless to use a long name here:
for (int i = 0; i < rooms.Count; i++)
{
rooms[i] = new Room();
}But in general, they are a PITA!
"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!
for (int room = 0, roomCount = rooms.Count; room < roomCount; room++)
{
rooms[room] = new Room();
}room can be replaced by pos (or anything more meaningful); anyway: anything not like this as a code will not go live for me... Besides this point, when I had to manage core reviews, I dit it with the whole team so anybody could have seen and emphasis on different things and not only what I wanted to pinpoint. One point missing, is that code reviews in 2021 aren't the same as those done in the 1990: now we have tools like R# Shart (no publicity intended) which are removing a lot of "discrepancies" (bugs), automating test tools, and so on. So now, code reviews are more about code's readability and maintenance. Because of these (hey folks, dont forget you're using Intellisense which types most of the code for you!), so definitively no excuse to have readable code.
-
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 + ccan confuse me if you write insteadmyobjectBlu = 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!
why? Because I am a software mechanic (engineer), and my first reaction is: "Who the F... wrote this?" You will learn over time that a good code review by your best colleagues will preserve your reputation, and people like me won't have that kind of reaction when we look at your code. "That's enough for me, please pass the album cover..." :cool::java:
~d~
-
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 + ccan confuse me if you write insteadmyobjectBlu = 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!
-
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 + ccan confuse me if you write insteadmyobjectBlu = 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!
I actually agree with you. The property name of "Value" makes sense as it's likely part of a larger class that has an outputted value. it's what I would look for browsing the class definition. The 'x' variable is used in such a short space, that's it's fine by me, since Calculation should have the definition for it's return value. a longer name would take longer to read, and add more noise to something so simple. A long time ago, I also subscribed to long variable names. like 'for' loops I used to us 'xloop', 'yloop', 'zloop'. but that was stupid, turned out it was way more 'noise' than worth while and started keeping the code cleaner like i, j, k..
for(int xloop =0;xloop
orfor(int i=0;i
I feel the second is way more easy to read. now if the variable was used over 10 or so lines, I would likely give it a much more meaningful name. but I look for clarity over hard rules.
Especially being a little dyslexic, the easier to read the better.I once worked with a developer that would have written it like:
for(int intCounterForAccumulationLoop=0;i
All three produce the same output, but the last one was by far the hardest to read and understand what was going on. -
Agreed. Can't stand them either. Just let me do my job. You don't need to be included. Go away and do your job. If you have a problem with my code, you are welcome to fix it on YOUR OWN time. And FYI, I might have a problem with your code too, but I'm not arrogant/rude/petty/anal enough to tell you. I'm not here to look over your shoulder, you need to be able to work on your own. If you are happy that the code works, the team are happy too. That felt good.
Maybe you work for a code effort where you are the only coder who will ever touch your code. Great for you. But if there's even a small chance your co-worker has to understand your code? Then it's not arrogant to expect it to make sense to a larger audience. It's common sense and good teamwork. I've been in this business a long damned time and I've seen what happens with and without reviews. I'll take the shop that does reviews, thanks. I would be arrogant if I thought my code should be untouchable.
-
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 + ccan confuse me if you write insteadmyobjectBlu = 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!
Put yourself in the imaginary position that you're someone who didn't write the code and has to understand what the author intended in order to be able to extend the functionality. It has to be understandable at the very first glance as if you were reading a book, because otherwise it's gonna take you hours because you get stuck every 1 1/2 lines and have to stop reading and try to guess: What does the author mean by "x"? The x axis? A cross? What does he mean by "cmp"? Component? Computer? Compare? From that point of view, it should be obvious that it's kind of rude to treat other programmers like that. No?
-
Put yourself in the imaginary position that you're someone who didn't write the code and has to understand what the author intended in order to be able to extend the functionality. It has to be understandable at the very first glance as if you were reading a book, because otherwise it's gonna take you hours because you get stuck every 1 1/2 lines and have to stop reading and try to guess: What does the author mean by "x"? The x axis? A cross? What does he mean by "cmp"? Component? Computer? Compare? From that point of view, it should be obvious that it's kind of rude to treat other programmers like that. No?
Exactly, for me the change make it harder to understand. Obviously people dont put themselves in my shoes.
A new .NET Serializer All in one Menu-Ribbon Bar Taking over the world since 1371!
-
Maybe you work for a code effort where you are the only coder who will ever touch your code. Great for you. But if there's even a small chance your co-worker has to understand your code? Then it's not arrogant to expect it to make sense to a larger audience. It's common sense and good teamwork. I've been in this business a long damned time and I've seen what happens with and without reviews. I'll take the shop that does reviews, thanks. I would be arrogant if I thought my code should be untouchable.
It's fine if someone doesn't have the confidence to write code without a supervisor. It shouldn't be forced on you. You should always write code for someone else, not yourself. And I'd imagine all pieces of code could be improved before *or after* someone "checks" it regardless. It would seem more arrogant to think other people's code needs fixing. I prefer trusting my team and their abilities. I don't and never will like code reviews. But you shouldn't take that personally.
-
It's fine if someone doesn't have the confidence to write code without a supervisor. It shouldn't be forced on you. You should always write code for someone else, not yourself. And I'd imagine all pieces of code could be improved before *or after* someone "checks" it regardless. It would seem more arrogant to think other people's code needs fixing. I prefer trusting my team and their abilities. I don't and never will like code reviews. But you shouldn't take that personally.
