Goto here?
On 9/16/2011 at 9:13 PM, wood_brian said:I think that has problems. If the first call to Write returns less than the segment size, the second call to Write won't be aligned properly. This could lead to incorrect data being sent...
Yep it's missing an "if". Although I think the real problem is that if the first call returns less than the segment size, and also less than "all", then it failed and we don't catch the failure. If it doesn't return less than "all", then the loop isn't run anyway, 'cause we're done writing .
But the point is that with a little bit of restructure it's every bit as readable as before (significantly, it's readable enough for the logic flaw to jump out at you), with simpler flow control, and no temptation to use a "goto".
In short, no need for "goto" here.
On 9/16/2011 at 9:13 PM, wood_brian said:I'm not keen on the std::min use there.
What is it about that use that you don't like?
But the point is that with a little bit of restructure it's every bit as readable as before (significantly, it's readable enough for the logic flaw to jump out at you), with simpler flow control, and no temptation to use a "goto".
In short, no need for "goto" here.
It might be more readable, but that doesn't matter if it's wrong. I think latent already posted working solution at post #3.
I read your code five times before I found the goto.
Given that it hurts readability that much, I'd say stick with your current solution. There is no gain to adding non-obvious control flow in this situation.
Funny I saw it instantly. But then Im used to using gotos all the time.
It usually helps to have the label name all it CAPS to make it stand out more (meaningfully worded labels are hard to misunderstand)
I never seem to consider a single goto near a procedures exit as 'convoluted' (again maybe Ive seen ALOT more really convoluted code including
alot where NOT using goto s causes very deep ugly indentations of layers of escape logic instead.
On 9/20/2011 at 3:53 AM, Ftn said:It might be more readable...
Although I certainly never claimed it was. Readability is at least partially dependent on the reader, and my code was in a different style than the OP's.
On 9/20/2011 at 3:53 AM, Ftn said:...but that doesn't matter if it's wrong.
Of course it does.
Easily readable code which is broken is readily fixed, the error in my code being case-in-point. Working code which is unreadable, on the other hand, will eventually break and prove most resistant to maintenance. Readability matters - in a sense, almost more than functionality.
But anyway, neither the simplified flow control, nor the readability of the code, would be compromised by adding the needed "if". So the fact that the code is "wrong" is all but irrelevant to the improvements it's getting at - just like the "working solution" already posted.
Easily readable code which is broken is readily fixed, the error in my code being case-in-point. Working code which is unreadable, on the other hand, will eventually break and prove most resistant to maintenance. Readability matters - in a sense, almost more than functionality.
I agree with you that readability matters a lot. I actually sketched some SocketWriter class to reply in this, it would make this function pretty easy to digest (and the a lot of this SendBuffer class much more simpler). But I decided not to post, because it would be better for SendBuffer to own instance of SocketWriter and I don't know how the rest of the class looks like..
