|
We don't do code reviews where I work. Go ahead, keep throwing up; I'll wait.
1. I don't like the goto. I know some folks like them for error exit handling when you're not using exceptions, but I'm not one of them.
2. If the ep symbol is intended to be a label, it's certainly poorly placed.
3. There's nothing inherently wrong in the numerous nested for loops. It depends upon their purpose.
3.1. If the nested loops are simple indices in multi-dimensional data performing a simple task, it might be the most concise way to accomplish that task. Adding layers of abstraction to remove the nesting might complicate the logic unnecessarily, especially if that's the only reason for adding the layer.
3.2. If 3.1 is not the case, the loops are certainly a code smell since they imply accessing multiple levels of detail from a single scope.
3.3. The iteration values i1, i2, etc. are poorly named.
4. I also don't like the return embedded in the loops. I don't have a problem using break or continue to exit a loop early, but that keeps the range of the 'goto' to the body of the loop, making control flow analysis simpler.
Software Zen: delete this;
|
|
|
|
|
OriginalGriff wrote: Your thoughts
The label 'ep' probably means exit point. The only thing I can think of is that the author is following the single-entry single-exit[^] code standard.
I've worked on teams that tried to adhere to this policy. I've had some of my code rejected by reviewers with the reason of: "Refactor to single exit". That code is in your operating system.
Best Wishes,
-David Delaune
|
|
|
|
|
We don't have code reviews but I would have one with the group just to point out how horrendous that is.
"They have a consciousness, they have a life, they have a soul! Damn you! Let the rabbits wear glasses! Save our brothers! Can I get an amen?"
|
|
|
|
|
Commenting the ending brace shows potential.
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
|
|
|
|
|
No, simply for nesting / length. I'm sure well-named functions could be pulled out, which would decrease the cognitive load on the reader, especially someone new to the code, which includes the original author in about a month or two.
------------------------------------------------
If you say that getting the money
is the most important thing
You will spend your life
completely wasting your time
You will be doing things
you don't like doing
In order to go on living
That is, to go on doing things
you don't like doing
Which is stupid.
|
|
|
|
|
Pascal, for instance, will not allow you to hyperspace into a scope like that. for good reason.
the only useful scenario for this "technique" is where you have the expressions after the closing brace of the inner loops. that way either you execute all the looping which creates some multi dimensional computation or you jump with goto inside the most inner loop and execute a liner computation, once for every loop.
but, in this case i fail to comprehend the reason for jumping at the tail of the loops.
equally puzzled by the most inner return.
this looks like is some kind of a trick interview question...
it would be interesting to know what is the purpose of this "device"?
|
|
|
|
|
|
That was my very first program in the 1960s. It was a FORTRAN loop to calculate Chess moves. My Dad punched it into cards and had it evaluated, and said it would take more years to compute than there are stars in the Universe.
|
|
|
|
|
In my career, I actually had to review a piece of code similar to this. My approach was to turn the code review into a teaching moment. I had the developer explain the problem they were trying to solve, what solutions they considered and why they chose this one.
The developer ended up reworking the code and produced a much-improved solution. More importantly, though the developer never sent code like this in for code review again.
|
|
|
|
|
Not sure what I find worse, the amount of nested for's or the use of goto.
To alcohol! The cause of, and solution to, all of life's problems - Homer Simpson
Our heads are round so our thoughts can change direction - Francis Picabia
|
|
|
|
|
Just the nesting of loops makes me cringe. Ouch. I have written software for biotech companies creating chemistry applications, libraries, and drivers and see this often. Comparing, subtracting, etc. x,y,z points in one group of data to another group of data, we get this awful type of code. But, that’s what the customer wants us to do. And he wants it better, faster, and cheaper.
|
|
|
|
|
|
CROSSWORD CLUE 
"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!
|
|
|
|
|
Yes... I thought that one wouldn't take long!
|
|
|
|
|
I quite like that one actually 
|
|
|
|
|
Is it Pandemic or other reasons that CP members traffic seems reduced? (feels like less active members now then it used to be 5-10 years back.)
No data - my personal observation which could be wrong
|
|
|
|
|
Detergent carton was a place to argue (7)
|
|
|
|
|
Neh - Don't think people were active just for soap-box.
|
|
|
|
|
Sandeep Mewara wrote: Don't think people were active just for soap-box.
Some were. But not enough to make that much of a difference.
Social Media - A platform that makes it easier for the crazies to find each other.
Everyone is born right handed. Only the strongest overcome it.
Fight for left-handed rights and hand equality.
|
|
|
|
|
'Other reasons', I believe.
|
|
|
|
|
Sandeep Mewara wrote: feels like less active members
Fewer not less. 
|
|
|
|
|
Quote: Fewer not less.
Poems, no less!
|
|
|
|
|
|
Ironically the politics of CP have caused some to go away.
Social Media - A platform that makes it easier for the crazies to find each other.
Everyone is born right handed. Only the strongest overcome it.
Fight for left-handed rights and hand equality.
|
|
|
|
|
ZurdoDev wrote: politics of CP
|
|
|
|
General 
News 
Suggestion 
Question 
Bug 
Answer 
Joke 
Praise 
Rant 
Admin 
Submit an article or tip
