Code reviews can make or break your team
-
This is what good code-review should look like: [^], to be followed-up, of course, by the burning of the apostates.
«I want to stay as close to the edge as I can without going over. Out on the edge you see all kinds of things you can't see from the center» Kurt Vonnegut.
-
-
Yes we all do. One thing I hate is not putting spaces between operators. This
for(rg=rmin;rg
for me is not a row of code, it is a single long word. Thisfor(rg = rmin; rg < rmax - 3; rg++){medie[rg] = (double) bottProfile[rg] + bottProfile[rg + 1] + bottProfile[rg + 2])/6.0;}
is a line of code.
Then I have a colleague who wrote a single huge CPP of 2700 rows of code like this. :mad::mad::mad:
GCS d--- s-/++ a- C++++ U+++ P- L- E-- W++ N++ o+ K- w+++ O? M-- V? PS+ PE- Y+ PGP t++ 5? X R++ tv-- b+ DI+++ D++ G e++>+++ h--- ++>+++ y+++* Weapons extension: ma- k++ F+2 X
If you think 'goto' is evil, try writing an Assembly program without JMP. -- TNCaver
"When you have eliminated the JavaScript, whatever remains must be an empty page." -- Mike Hankey
-
Yes we all do. One thing I hate is not putting spaces between operators. This
for(rg=rmin;rg
for me is not a row of code, it is a single long word. Thisfor(rg = rmin; rg < rmax - 3; rg++){medie[rg] = (double) bottProfile[rg] + bottProfile[rg + 1] + bottProfile[rg + 2])/6.0;}
is a line of code.
Then I have a colleague who wrote a single huge CPP of 2700 rows of code like this. :mad::mad::mad:
GCS d--- s-/++ a- C++++ U+++ P- L- E-- W++ N++ o+ K- w+++ O? M-- V? PS+ PE- Y+ PGP t++ 5? X R++ tv-- b+ DI+++ D++ G e++>+++ h--- ++>+++ y+++* Weapons extension: ma- k++ F+2 X
If you think 'goto' is evil, try writing an Assembly program without JMP. -- TNCaver
"When you have eliminated the JavaScript, whatever remains must be an empty page." -- Mike Hankey
Never mind the spaces, I'm breaking out the napalm canisters over the lack of newlines. X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X|
-
Never mind the spaces, I'm breaking out the napalm canisters over the lack of newlines. X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X| X|
That is the second thing that drives me wild. Sometimes even I put several short lines in a single line: it may (or may not) have sense and actually improve readability. Of course this is not such a case but consider that the function is very long and not breakable. But no spaces totally break my token recognition. Wherever I can I try to keep all the tokens in columns, unless it is stupid to do so (fully specified class names can be pretty long).
GCS d--- s-/++ a- C++++ U+++ P- L- E-- W++ N++ o+ K- w+++ O? M-- V? PS+ PE- Y+ PGP t++ 5? X R++ tv-- b+ DI+++ D++ G e++>+++ h--- ++>+++ y+++* Weapons extension: ma- k++ F+2 X If you think 'goto' is evil, try writing an Assembly program without JMP. -- TNCaver "When you have eliminated the JavaScript, whatever remains must be an empty page." -- Mike Hankey
-
That is the second thing that drives me wild. Sometimes even I put several short lines in a single line: it may (or may not) have sense and actually improve readability. Of course this is not such a case but consider that the function is very long and not breakable. But no spaces totally break my token recognition. Wherever I can I try to keep all the tokens in columns, unless it is stupid to do so (fully specified class names can be pretty long).
GCS d--- s-/++ a- C++++ U+++ P- L- E-- W++ N++ o+ K- w+++ O? M-- V? PS+ PE- Y+ PGP t++ 5? X R++ tv-- b+ DI+++ D++ G e++>+++ h--- ++>+++ y+++* Weapons extension: ma- k++ F+2 X If you think 'goto' is evil, try writing an Assembly program without JMP. -- TNCaver "When you have eliminated the JavaScript, whatever remains must be an empty page." -- Mike Hankey
den2k88 wrote:
Sometimes even I put several short lines in a single line: it may (or may not) have sense and actually improve readability. Of course this is not such a case but consider that the function is very long and not breakable.
My feelings on that range from neutral/mild distaste in trivial cases, (eg
int i = 0; int j = 0; ink k = 0;) mostly because anything exceptional causes me to have to slow down and take a 2nd look. I'm extremely strongly against it in the case of conditionals/loops; because without a blank line afterward to cleanly separate it from whatever follows it's too prone to confusion and a blank line is too easily lost.den2k88 wrote:
Wherever I can I try to keep all the tokens in columns,
Many years back I used to do that a lot; but have thrown in the towel because doing so is incompatible with all the auto-formatters I've used and I find them to be more generally useful. I'll still do it occasionally, but it's very much a by exception thing now.
Did you ever see history portrayed as an old man with a wise brow and pulseless heart, waging all things in the balance of reason? Is not rather the genius of history like an eternal, imploring maiden, full of fire, with a burning heart and flaming soul, humanly warm and humanly beautiful? --Zachris Topelius Training a telescope on one’s own belly button will only reveal lint. You like that? You go right on staring at it. I prefer looking at galaxies. -- Sarah Hoyt
-
den2k88 wrote:
Sometimes even I put several short lines in a single line: it may (or may not) have sense and actually improve readability. Of course this is not such a case but consider that the function is very long and not breakable.
My feelings on that range from neutral/mild distaste in trivial cases, (eg
int i = 0; int j = 0; ink k = 0;) mostly because anything exceptional causes me to have to slow down and take a 2nd look. I'm extremely strongly against it in the case of conditionals/loops; because without a blank line afterward to cleanly separate it from whatever follows it's too prone to confusion and a blank line is too easily lost.den2k88 wrote:
Wherever I can I try to keep all the tokens in columns,
Many years back I used to do that a lot; but have thrown in the towel because doing so is incompatible with all the auto-formatters I've used and I find them to be more generally useful. I'll still do it occasionally, but it's very much a by exception thing now.
Did you ever see history portrayed as an old man with a wise brow and pulseless heart, waging all things in the balance of reason? Is not rather the genius of history like an eternal, imploring maiden, full of fire, with a burning heart and flaming soul, humanly warm and humanly beautiful? --Zachris Topelius Training a telescope on one’s own belly button will only reveal lint. You like that? You go right on staring at it. I prefer looking at galaxies. -- Sarah Hoyt
Well some code like
if (condition) { flag = false; break; }
if (condition2) { otherflag = false; break; }is actually (in my opinion) more readable like this because it is actually one / two short operations that are extremely correlated and stay very wall on one line. But actually this is the only case I put more instructions on a single line - I especialli avoid the "int i = 0; int j; int k = 0;" case because is is less flexible to add/remove/rename variables. I tweaked VisualAssistX so that now it doesn't autoformat as I don't like, only the VB6 IDE autoformats without possibility of turning it off.
GCS d--- s-/++ a- C++++ U+++ P- L- E-- W++ N++ o+ K- w+++ O? M-- V? PS+ PE- Y+ PGP t++ 5? X R++ tv-- b+ DI+++ D++ G e++>+++ h--- ++>+++ y+++* Weapons extension: ma- k++ F+2 X If you think 'goto' is evil, try writing an Assembly program without JMP. -- TNCaver "When you have eliminated the JavaScript, whatever remains must be an empty page." -- Mike Hankey
