You'll be glad to know! [modified]
-
Yeah, but kinda depends on what's after the loop though. Could replace with a
break;but then you might end up having to do anotherif (points.Count < 3)Sometimes getting the hell out of dodge is the best option. Depends, it always just depends. As long as it's clear what's going on, right? 1001 ways to skin a cat (although I would never do that, unless I was beyond hunger, or it was a Kobe cat!) :)
- S 50 cups of coffee and you know it's on! Code, follow, or get out of the way.
Steve Echols wrote:
Could replace with a break;
Ug, I hate 'breaks' too. I guess I've never seen a good reason to break the flow of code with any of those things. I'd just add another variable and throw it in with the conditional in the for loop and then check it again after the loop if necessary (like you say - depends on what else is going on). Adds more code, but the conditional variable at least makes it obvious why the loop is being aborted, and the check for it goes with the loop construct where it should be anyway (all the reasons for stopping the loop are in one place).
var result=null;
bool enoughPoints=true;
for (int i = max; i >= min && enoughPoints; i--)
{
bool removePoint = false;
var v1 = new Vector2D(points[i > 0 ? i - 1 : points.Count - 1], points[i]);
var v2 = new Vector2D(points[i], points[i < points.Count - 1 ? i + 1 : 0]);
if (v1.SquareNorm <= MINL || v2.SquareNorm <= MINL)
{
removePoint = true;
}
else
{
v1 = v1.Normalize(); // divide by zero if square norm is 0 (test above)
v2 = v2.Normalize();
var z = v1 ^ v2;
if (Math.Abs(z) <= minsin && v1 * v2 < 0)
{
removePoint = true;
}
}
if (removePoint)
{
points.RemoveAt(i);
if (points.Count < 3)
{
enoughPoints=false;
}
}
}
if(enoughPoints)
{
...some other code that may affect 'result'
}
return result;Cheers, Drew.
-
My reply is that I should not have reply to all this post, this is going nowhere! Particularly all those alternative are nothing new, so it's mostly boring... Just for the record I write an average of 1 goto a year. I'm happy with it and last I look back at 10 years old code with goto, I still find it much easier to understand this way!
A train station is where the train stops. A bus station is where the bus stops. On my desk, I have a work station.... _________________________________________________________ My programs never have bugs, they just develop random features.
-
Super Lloyd wrote:
My reply is that I should not have reply to all this post, this is going nowhere!
Heh, then why are you replying? Cheers, Drew.
Oops, I mean "my problem"! But... you're right! :laugh: (I'm still learning) .....
A train station is where the train stops. A bus station is where the bus stops. On my desk, I have a work station.... _________________________________________________________ My programs never have bugs, they just develop random features.
-
Steve Echols wrote:
Could replace with a break;
Ug, I hate 'breaks' too. I guess I've never seen a good reason to break the flow of code with any of those things. I'd just add another variable and throw it in with the conditional in the for loop and then check it again after the loop if necessary (like you say - depends on what else is going on). Adds more code, but the conditional variable at least makes it obvious why the loop is being aborted, and the check for it goes with the loop construct where it should be anyway (all the reasons for stopping the loop are in one place).
var result=null;
bool enoughPoints=true;
for (int i = max; i >= min && enoughPoints; i--)
{
bool removePoint = false;
var v1 = new Vector2D(points[i > 0 ? i - 1 : points.Count - 1], points[i]);
var v2 = new Vector2D(points[i], points[i < points.Count - 1 ? i + 1 : 0]);
if (v1.SquareNorm <= MINL || v2.SquareNorm <= MINL)
{
removePoint = true;
}
else
{
v1 = v1.Normalize(); // divide by zero if square norm is 0 (test above)
v2 = v2.Normalize();
var z = v1 ^ v2;
if (Math.Abs(z) <= minsin && v1 * v2 < 0)
{
removePoint = true;
}
}
if (removePoint)
{
points.RemoveAt(i);
if (points.Count < 3)
{
enoughPoints=false;
}
}
}
if(enoughPoints)
{
...some other code that may affect 'result'
}
return result;Cheers, Drew.
Depends :) As long as there's nothing after the if (removePoints) block that relies on having enough points, that works for me! Just gotta structure everything right, to minimize checks. Dang depends! Starting to feel like I'm about to have a coding accident now! :-D
- S 50 cups of coffee and you know it's on! Code, follow, or get out of the way.
-
Depends :) As long as there's nothing after the if (removePoints) block that relies on having enough points, that works for me! Just gotta structure everything right, to minimize checks. Dang depends! Starting to feel like I'm about to have a coding accident now! :-D
- S 50 cups of coffee and you know it's on! Code, follow, or get out of the way.
-
:thumbsup: Great minds think alike!
- S 50 cups of coffee and you know it's on! Code, follow, or get out of the way.
-
Yesterday I wrote no less than 2 gotos in my C# code! [Edit2] This guy[^] found a satisfying refactoring! One which doesn't tax my (limited brain) memory with nested block, additional variable, additional test, increased cyclomatic complexity! (In fact the cyclomatic complexity is reduced by 1, I think) Well done! :thumbsup: :cool: :-D [EDIT] For your information the real code is below. So far no one has come up with a non goto version which can beat the goto version on any of those 4 criteria: - number of line of code (less) - number of if (less) - number of nested block (less) - number of variables (less) -- code --
for (int i = max; i >= min; i--)
{
var v1 = new Vector2D(points[i > 0 ? i - 1 : points.Count - 1], points[i]);
var v2 = new Vector2D(points[i], points[i < points.Count - 1 ? i + 1 : 0]);
if (v1.SquareNorm <= MINL || v2.SquareNorm <= MINL)
goto RemovePoint;
v1 = v1.Normalize(); // divide by zero if square norm is 0 (test above)
v2 = v2.Normalize();
var z = v1 ^ v2;
if (Math.Abs(z) <= minsin && v1 * v2 < 0)
goto RemovePoint;
continue;
RemovePoint: ;
points.RemoveAt(i);
if (points.Count < 3)
return null;
}A train station is where the train stops. A bus station is where the bus stops. On my desk, I have a work station.... _________________________________________________________ My programs never have bugs, they just develop random features.
modified on Friday, September 10, 2010 10:53 AM
Fast to code, easy to understand (despite the fact that the code is disastrously lacking in comments), and better performance than most alternatives. Why on Earth wouldn't you use a goto for that? The "approved" Java equivalent would create at least one factory to construct at least two additional objects to do the same thing (and no-one would be able to figure out what was going on without clicking through to the various constructors and back half a dozen times). Too many developers are D&D or LoR fans -- The Lord High Muck-a-Muck Wizard says Useth Not The Goto!, and his word must be obeyed. I'm still waiting for someone to explain to me why code that isn't broken should be fixed.
I wanna be a eunuchs developer! Pass me a bread knife!
-
Fast to code, easy to understand (despite the fact that the code is disastrously lacking in comments), and better performance than most alternatives. Why on Earth wouldn't you use a goto for that? The "approved" Java equivalent would create at least one factory to construct at least two additional objects to do the same thing (and no-one would be able to figure out what was going on without clicking through to the various constructors and back half a dozen times). Too many developers are D&D or LoR fans -- The Lord High Muck-a-Muck Wizard says Useth Not The Goto!, and his word must be obeyed. I'm still waiting for someone to explain to me why code that isn't broken should be fixed.
I wanna be a eunuchs developer! Pass me a bread knife!
Exactly and well said! :-D
A train station is where the train stops. A bus station is where the bus stops. On my desk, I have a work station.... _________________________________________________________ My programs never have bugs, they just develop random features.
-
Fast to code, easy to understand (despite the fact that the code is disastrously lacking in comments), and better performance than most alternatives. Why on Earth wouldn't you use a goto for that? The "approved" Java equivalent would create at least one factory to construct at least two additional objects to do the same thing (and no-one would be able to figure out what was going on without clicking through to the various constructors and back half a dozen times). Too many developers are D&D or LoR fans -- The Lord High Muck-a-Muck Wizard says Useth Not The Goto!, and his word must be obeyed. I'm still waiting for someone to explain to me why code that isn't broken should be fixed.
I wanna be a eunuchs developer! Pass me a bread knife!
GOTOs complicate code. They make what is clearly understandable structure into a possible back and forth jumble. They essentially turn a tree into a graph, which is much more difficult to navigate. With a GOTO, you can jump to anywhere in the program that has a label (in C#, that might be limited to the current method, but I'm not sure), which makes it hard to follow. With a clear code flow (conditional blocks, looping blocks, and so on), things are easier to follow. GOTOs can be useful in certain cases where performance is of extreme importance, but then again so can assembly instructions. The purpose of using a high-level language is to escape the complexity that comes with maintaining low-level constructs such as GOTO. Like I said, GOTOs can be useful to improve performance in very limited scenarios, but that doesn't mean they should be used when an alternative exists that doesn't cause strange code flow.
-
GOTOs complicate code. They make what is clearly understandable structure into a possible back and forth jumble. They essentially turn a tree into a graph, which is much more difficult to navigate. With a GOTO, you can jump to anywhere in the program that has a label (in C#, that might be limited to the current method, but I'm not sure), which makes it hard to follow. With a clear code flow (conditional blocks, looping blocks, and so on), things are easier to follow. GOTOs can be useful in certain cases where performance is of extreme importance, but then again so can assembly instructions. The purpose of using a high-level language is to escape the complexity that comes with maintaining low-level constructs such as GOTO. Like I said, GOTOs can be useful to improve performance in very limited scenarios, but that doesn't mean they should be used when an alternative exists that doesn't cause strange code flow.
So you're saying that gotos, like anything else, can be used badly. That doesn't mean that they have to be (and nor does anything else). If we exclude everything from a language because it can be used badly, there won't be a Hell of a lot left.
I wanna be a eunuchs developer! Pass me a bread knife!
-
Yesterday I wrote no less than 2 gotos in my C# code! [Edit2] This guy[^] found a satisfying refactoring! One which doesn't tax my (limited brain) memory with nested block, additional variable, additional test, increased cyclomatic complexity! (In fact the cyclomatic complexity is reduced by 1, I think) Well done! :thumbsup: :cool: :-D [EDIT] For your information the real code is below. So far no one has come up with a non goto version which can beat the goto version on any of those 4 criteria: - number of line of code (less) - number of if (less) - number of nested block (less) - number of variables (less) -- code --
for (int i = max; i >= min; i--)
{
var v1 = new Vector2D(points[i > 0 ? i - 1 : points.Count - 1], points[i]);
var v2 = new Vector2D(points[i], points[i < points.Count - 1 ? i + 1 : 0]);
if (v1.SquareNorm <= MINL || v2.SquareNorm <= MINL)
goto RemovePoint;
v1 = v1.Normalize(); // divide by zero if square norm is 0 (test above)
v2 = v2.Normalize();
var z = v1 ^ v2;
if (Math.Abs(z) <= minsin && v1 * v2 < 0)
goto RemovePoint;
continue;
RemovePoint: ;
points.RemoveAt(i);
if (points.Count < 3)
return null;
}A train station is where the train stops. A bus station is where the bus stops. On my desk, I have a work station.... _________________________________________________________ My programs never have bugs, they just develop random features.
modified on Friday, September 10, 2010 10:53 AM
I wouldn't be trying to beat those criteria as they don't match the style that I prefer. I would instead refactor this to use a helper routine:
bool ShouldPointBeRemoved(List points, int i)
{
var v1 = new Vector2D(points[i > 0 ? i - 1 : points.Count - 1], points[i]);
var v2 = new Vector2D(points[i], points[i < points.Count - 1 ? i + 1 : 0]);
if (v1.SquareNorm <= MINL || v2.SquareNorm <= MINL)
return true;v1 = v1.Normalize(); v2 = v2.Normalize(); var z = v1 ^ v2; return (Math.Abs(z) <= minsin && v1 \* v2 < 0);}
Note: I used a List for the points, as you never specified a type. Once this helper routine is provided, the loop becomes:
for (int i = max; i >= min; i--)
{
if (ShouldPointBeRemoved(points, i))
{
points.RemoveAt(i);
if (points.Count < 3)
return null;
}
}I find that this refactoring is much easier to read than the original code, as the loop now clearly shows its intent. If you really don't like extra indent level in the loop, you could use a continue:
for (int i = max; i >= min; i--)
{
if (! ShouldPointBeRemoved(points, i))
continue;
points.RemoveAt(i);
if (points.Count < 3)
return null;
} -
I wouldn't be trying to beat those criteria as they don't match the style that I prefer. I would instead refactor this to use a helper routine:
bool ShouldPointBeRemoved(List points, int i)
{
var v1 = new Vector2D(points[i > 0 ? i - 1 : points.Count - 1], points[i]);
var v2 = new Vector2D(points[i], points[i < points.Count - 1 ? i + 1 : 0]);
if (v1.SquareNorm <= MINL || v2.SquareNorm <= MINL)
return true;v1 = v1.Normalize(); v2 = v2.Normalize(); var z = v1 ^ v2; return (Math.Abs(z) <= minsin && v1 \* v2 < 0);}
Note: I used a List for the points, as you never specified a type. Once this helper routine is provided, the loop becomes:
for (int i = max; i >= min; i--)
{
if (ShouldPointBeRemoved(points, i))
{
points.RemoveAt(i);
if (points.Count < 3)
return null;
}
}I find that this refactoring is much easier to read than the original code, as the loop now clearly shows its intent. If you really don't like extra indent level in the loop, you could use a continue:
for (int i = max; i >= min; i--)
{
if (! ShouldPointBeRemoved(points, i))
continue;
points.RemoveAt(i);
if (points.Count < 3)
return null;
}You know what? I think you are the only one so far who found a satisfying refactoring even from my perspective! I'm impressed! :-D :thumbsup: :rose: :omg: :cool: No extra variable, no extra if, no extra nested block... Even 1 less jump (continue has been removed) I.e. you even reduced the total cyclomatic complexity! Your approach is one I can pleasantly used, even with my limited (brain) memory, and even without being pushed to it!! :) (in fact I might even refactor my code this way Monday! :rolleyes: )
A train station is where the train stops. A bus station is where the bus stops. On my desk, I have a work station.... _________________________________________________________ My programs never have bugs, they just develop random features.
-
Maybe you an answer this other question. Somewhere I posted my real code and somewhere some posted the nested if version. Personally I found my version way simpler and maintainable. It has less line, less bracket, less variable a lower cyclomatic complexity. Now where does this "absolutely no goto" religion comes from? I just can't bring myself to prefer the nested if extra variable more lines of code version.... :omg: Anyway, I can see this is going no where. I did learn something, it's pointless to post against people belief, nothing came out of it! I haven't change my opinion, nor did they! and I'm upset!
A train station is where the train stops. A bus station is where the bus stops. On my desk, I have a work station.... _________________________________________________________ My programs never have bugs, they just develop random features.
Super Lloyd wrote:
Personally I found my version way simpler and maintainable. It has less line, less bracket, less variable a lower cyclomatic complexity.
Which is weird, because I prefer brackets (or braces) as I can then visually compartmentalize what the code is doing.
Super Lloyd wrote:
I haven't change my opinion, nor did they! and I'm upset!
Well, that's why programming is like art. Everyone has an opinion, especially when aesthetics is involved (and goto falls in that category, IMO), and it really is very hard to determine whether there is an underlying truth and what it is. Marc
-
I wouldn't be trying to beat those criteria as they don't match the style that I prefer. I would instead refactor this to use a helper routine:
bool ShouldPointBeRemoved(List points, int i)
{
var v1 = new Vector2D(points[i > 0 ? i - 1 : points.Count - 1], points[i]);
var v2 = new Vector2D(points[i], points[i < points.Count - 1 ? i + 1 : 0]);
if (v1.SquareNorm <= MINL || v2.SquareNorm <= MINL)
return true;v1 = v1.Normalize(); v2 = v2.Normalize(); var z = v1 ^ v2; return (Math.Abs(z) <= minsin && v1 \* v2 < 0);}
Note: I used a List for the points, as you never specified a type. Once this helper routine is provided, the loop becomes:
for (int i = max; i >= min; i--)
{
if (ShouldPointBeRemoved(points, i))
{
points.RemoveAt(i);
if (points.Count < 3)
return null;
}
}I find that this refactoring is much easier to read than the original code, as the loop now clearly shows its intent. If you really don't like extra indent level in the loop, you could use a continue:
for (int i = max; i >= min; i--)
{
if (! ShouldPointBeRemoved(points, i))
continue;
points.RemoveAt(i);
if (points.Count < 3)
return null;
}The real question why didn't I think of this one?!? Mm.. maybe a method line that bool method(List<T> list, int index) looks kind of funny, the index parameter...
A train station is where the train stops. A bus station is where the bus stops. On my desk, I have a work station.... _________________________________________________________ My programs never have bugs, they just develop random features.
-
Super Lloyd wrote:
Personally I found my version way simpler and maintainable. It has less line, less bracket, less variable a lower cyclomatic complexity.
Which is weird, because I prefer brackets (or braces) as I can then visually compartmentalize what the code is doing.
Super Lloyd wrote:
I haven't change my opinion, nor did they! and I'm upset!
Well, that's why programming is like art. Everyone has an opinion, especially when aesthetics is involved (and goto falls in that category, IMO), and it really is very hard to determine whether there is an underlying truth and what it is. Marc
Marc Clifton wrote:
especially when aesthetics is involved (and goto falls in that category, IMO)
I'm glad you take it this way. Seems about right to me! And, strangely enough, all the other advised refactoring so far, I found they requires more (brain memory) to process. My memory is no good and I find it hard to cope with a "more elegant solution which requires more memory to understand" (this might be due to different thought processing strategy, mind you!). Anyway, last but not least, just so you know, not that it matter much, but this guy[^] just found a refactoring I find good! It doesn't requires any extra variable, nested block, if statement! In fact it also remove the continue.
A train station is where the train stops. A bus station is where the bus stops. On my desk, I have a work station.... _________________________________________________________ My programs never have bugs, they just develop random features.
-
Marc Clifton wrote:
especially when aesthetics is involved (and goto falls in that category, IMO)
I'm glad you take it this way. Seems about right to me! And, strangely enough, all the other advised refactoring so far, I found they requires more (brain memory) to process. My memory is no good and I find it hard to cope with a "more elegant solution which requires more memory to understand" (this might be due to different thought processing strategy, mind you!). Anyway, last but not least, just so you know, not that it matter much, but this guy[^] just found a refactoring I find good! It doesn't requires any extra variable, nested block, if statement! In fact it also remove the continue.
A train station is where the train stops. A bus station is where the bus stops. On my desk, I have a work station.... _________________________________________________________ My programs never have bugs, they just develop random features.
Super Lloyd wrote:
this might be due to different thought processing strategy, mind you!
Exactly, and that's a reality that has to be accepted. It also makes working with people in teams challenging, especially when we try to change each other's coding style, not realizing that individually, we're coding in a style comfortable and best suited for ourselves, but not necessarily comfortable or best suited for someone else that also has to work with the same code. Complicated! It'd be a really useful advancement in programming technology if the tool could be taught what our coding comfort/styles are and adjust the code to meet each of our individual needs. Wouldn't that be something! Marc
-
Super Lloyd wrote:
this might be due to different thought processing strategy, mind you!
Exactly, and that's a reality that has to be accepted. It also makes working with people in teams challenging, especially when we try to change each other's coding style, not realizing that individually, we're coding in a style comfortable and best suited for ourselves, but not necessarily comfortable or best suited for someone else that also has to work with the same code. Complicated! It'd be a really useful advancement in programming technology if the tool could be taught what our coding comfort/styles are and adjust the code to meet each of our individual needs. Wouldn't that be something! Marc
I heard that the team for VS2024 would be working on it! Having been beaten to it by the Resharper plugin!
A train station is where the train stops. A bus station is where the bus stops. On my desk, I have a work station.... _________________________________________________________ My programs never have bugs, they just develop random features.
