code review
-
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!
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"
-
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
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.
-
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 :-)
-
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!
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.
-
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 :-)
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
-
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
while (never)
A new .NET Serializer All in one Menu-Ribbon Bar Taking over the world since 1371!
-
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
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)'.
-
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 :-)
Compared with
while(1), (8 keystrokes)for(ever)has 9 keystrokes and is a distant third place fromfor(;;)at only 7 keystrokes :laugh: Sorry I'm lazy; I was born that way.Mircea
-
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 :-)
-
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 :-)
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.
-
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!
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
-
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"
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!
-
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.
-
Compared with
while(1), (8 keystrokes)for(ever)has 9 keystrokes and is a distant third place fromfor(;;)at only 7 keystrokes :laugh: Sorry I'm lazy; I was born that way.Mircea
-
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.
-
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.
-
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!
-
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 :-)
trønderen wrote:
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 (1)'
(Changed the 0 to 1 per your earlier replies) Lint may disagree with his assertion that while(1) {} is the only and preferred way to write a loop - I have had it prefer the for(;;) construct and both are versions that are easily recognised. I wouldn't personally be a fan of "#define ever ;;" as it "feels" fragile both in requiring the definition be included consistently and the risk someone may one day want to name a variable or function as that. Would be something for an interesting private discussion rather than "show and tell" at a scrum meeting, however!
-
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!
Years ago, I had the same. A colleague wrote an export routine to export a number of tables to an XML file. The tables where already loaded in memory. He came up with an over the top export routine of about 20.000 lines of code. He had been working on it for 3 weeks. I had to do the code review. I looked at it and rejected ALL of it. (And I wasn't revengeful) I told him we could do it with ONE little statement. (ds.GetXml() - where ds is the already loaded DataSet):cool: He never spoke to me again until I left the company. :-D
