I don't like code reviews
-
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. -
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.
-
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
-
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 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
-
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, notEnergy = 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!
Super Lloyd wrote:
so we disagree to disagree then?
No. If I was the boss and an employee used
Calculateas 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++
-
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 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; -
Super Lloyd wrote:
so we disagree to disagree then?
You agree to disagree. If you disagree to disagree, then you agree.
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!
-
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;"depends on context" Amen !
«The mind is not a vessel to be filled but a fire to be kindled» Plutarch
-
In 23 years, I've never had my code reviewed. I feel lucky! :)
"Go forth into the source" - Neal Morse "Hope is contagious"
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
-
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!
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
-
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
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!
-
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, notEnergy = 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!
We have a rule, if an acronym is well known, then we use it (eg url rather than universal resource locator), otherwise we spell out the full words. A few years ago we where working on a system which had a limitation on some names, but we needed those names to be descriptive of the keys. We ended up with a jumble of 3 letter acronyms everywhere. Sure enough, the parts we touched frequently where easy enough to deal with, but gee looking at some of the sections which remained fairly static was a nightmare to determine what was going on and what held what data.
-
We have a rule, if an acronym is well known, then we use it (eg url rather than universal resource locator), otherwise we spell out the full words. A few years ago we where working on a system which had a limitation on some names, but we needed those names to be descriptive of the keys. We ended up with a jumble of 3 letter acronyms everywhere. Sure enough, the parts we touched frequently where easy enough to deal with, but gee looking at some of the sections which remained fairly static was a nightmare to determine what was going on and what held what data.
you missed the part where I was talking about a variable.. inside a method...
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!
Right, tell your maths lecturer to use long variable names, instead of x and y.
Nothing succeeds like a budgie without teeth.
-
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, notEnergy = 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!
In an IT class I had to help someone who was a competent coder, but had a similar problem (he was dyslexic). The course specified that you should use descriptive variable names (fortunately not hungarian though). We reached a compromise: he could use short variable names wherever possible (eg inside small functional units of code - as per your example) but had to add a comment at the start of the code that explained what the variables represented. This had to be only in code that could fit on the screen so it could be read in one hit. Ignoring - for now - the fact that no developer I've ever met (including me!) is good at keeping comments in step with the code, nevertheless this seemed a workable compromise: He could write code he could follow and there was at least a reasonable chance that the comments would give a good hint as to what was going on.
-
you missed the part where I was talking about a variable.. inside a method...
A new .NET Serializer All in one Menu-Ribbon Bar Taking over the world since 1371!
I've also seen enough var x = 1; var y = 2; var z = 3; type code in methods to know that single letter / acronym variable names can be bad, especially that method has grown a little too big. Whilst variable names should never be war and piece, they should be somewhat descriptive enough so that as a developer you know what values you're likely to see in there, and how they should be used. That said, I'll happily use for (var i = 0; i < 10; i++) {} without thinking twice, so you know I'm not 100% dedicated to the no small variable names cause.
-
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 personally like descriptive names. atoi is way less readable, than StringToInt, for example. But that's a library function (I'm working polyglot, but not everyone is), what about own functions? When I write code, I like writing it in a way that makes clear what it does. Sure, I could litter it with comments, but too much of a good thing is real. So I have functions named DecodeL7 (with L standing for "layer" which, I admit, may be not explanatory enough). My variable to hold the L7 buffer is named accordingly, L7Buffer. Inside the DecodeL7 function, the variable name is just L7, it being a buffer is kinda obvious from context. The example above however is a bad one. Calculation() does exactly 0 to tell the maintainer (who may be another person, or oneself in 10 years) what the code does, so it may just as well be DoIt() or frob() or just f(). With every modern programming environment supporting unicode, there's a plethora of one-letter identifiers. ä(µ) isn't readable though. At all. If it's not code I'm working on daily, I know I'll forget what it's doing after taking care of another product for a month. As for my reading habits, I indeed parse whole words at once, both in code and in text. As for useless code review comments, a co-worker of mine is the kind of guy saying, almost verbatim, "I've learned it that way half a century ago and I'll never learn anything ever again". Needless to say, his comments during code reviews are utter shite.
-
Right, tell your maths lecturer to use long variable names, instead of x and y.
Nothing succeeds like a budgie without teeth.
Haha... 25 years and 20,000km too far and too late
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!
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.
-
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!
How about double Value => Calculation() * (flag ? 1 : 2); Now you don't need to deal with any variable names. =) // E