Just because a function is long does not mean it is bad. If the method you've pulled out is actually used in a number of places (instead of just the one location you pulled it from), then it may sense to turn it into a function. If it is only called from that one location then you've basically just made the code a bit harder to maintain for no real benefit.
I very much disagree with that statement. Breaking large functions down into clearly named smaller functions that each do a single thing is good. It aids in understanding while reducing the number of comments required, and makes code reuse easier in the future. It's not just about DRY.
If I feel like writing a comment to explain what a small section of code is doing, I usually just extract a well-named function instead.
I very much disagree with that statement. Breaking large functions down into clearly named smaller functions that each do a single thing is good. It aids in understanding while reducing the number of comments required, and makes code reuse easier in the future. It's not just about DRY.
Edit: I feel dumb, I'm not really adding anything new to this conversation.
I think i have a good example of a good usage of a goto in the program i am currently working on. Altrough the code
is rather long, you can really see how much this goto is usefull in this situation. This is the "master" function of
a dll, so that's why it's rather big, i stripped most of the code for simplicity reasons,
//-----------------------------------------------------------------------------
// Where all the magic's happen...
//-----------------------------------------------------------------------------
DWORD WINAPI ReadLoopThread(void *param)
{
// <initialisation code>
try
{
// put program in an infinite loop of reading and writing data
while(1)
{
// Exit the loop if we've been asked too...
if(ReadLoop.MustExitThread())
break;
// Is the socket ready to read?
if(pNetServer->CanRead()){
// Read the Header of the message
if(!ReadPacket.IsHeaderDoneProcessing()){
// <some code... read the socket>
// Are we done reading the header?
if(ReadPacket.GetHeaderIndx() == sizeof(PacketHeaderStruct)){
// Take action depeding on the message
switch(ReadPacket.Header.Msg)
{
case MSG_HANDSHAKING_REQUEST:
// Read the login data and return true if sucessfull...
if(OnHandshakeRequest(&ReadPacket, hWnd)){
// <some code...>
} else {
goto DisconnectJmp;
}
break;
case MSG_QTZ_IMG_REQUEST:
{
// <some code...>
if(SS.Size > 0){
// Send the data
if(!WriteData(SS.Buffers.QTZP.Compressed.GetBuffer(HeaderSize), SS.Size))
goto DisconnectJmp;
}
// <some code...>
}
break;
case MSG_MP1_IMG_REQUEST:
{
// <some code...>
if(SS.Size > 0){
// Send the data
if(!WriteData(SS.Buffers.MPEG.Encoded.GetBuffer(HeaderSize), SS.Size))
goto DisconnectJmp;
}
// <some code...>
}
break;
case MSG_INPUT_DATA:
{
// <some code...>
}
break;
}
I think i have a good example of a good usage of a goto in the program i am currently working on.
You can do cleanup with RAII instead.
DWORD WINAPI ReadLoopThread(void *param)
{
// <initialisation code>
Cleanup cl([&]() { CleanupReadLoopThread(hWnd, &VideoEncoder, &OriginalDesktopColor); });
try
{
// put program in an infinite loop of reading and writing data
...
for (int i=0; i<n && bContinue; ++i) {
switch (some_array) {
case 0:
//...
break;
case 1:
if (some_condition())
bContinue = false; // Break out of the loop, in a situation where `break' would just break out of the switch block.
break;
//...
}
}
//...
I have honestly never even considered goto here, and still will never.
L. Spiro
Fair enough. I was merely stating my opinion about your proposed solution. Your new solution would be the one I went with as well. I have not found myself needing to use goto since my days of QBasic.
I've used one goto in the say... 18 years I've been programming. It was a throwaway test app that I had to trigger cleanup code before exiting the function and had done it so slapdash that a finally block wouldn't deal with it; and it amused me to write such horrible code.
If you used a goto in production code I would fire you on the spot.
Well than you would be possibly firing Kenton Varda, Sanjay Ghemaway, Jeff Dean, and others... Oh wait, they work at Google so they don't need to work for someone as close-minded as you.
Denzel Morris (@drdizzy) :: Software Engineer :: SkyTech Enterprises, Inc.
"When men are most sure and arrogant they are commonly most mistaken, giving views to passion without that proper deliberation which alone can secure them from the grossest absurdities." - David Hume
[quote name='Telastyn' timestamp='1312667864' post='4845567']
I've used one goto in the say... 18 years I've been programming. It was a throwaway test app that I had to trigger cleanup code before exiting the function and had done it so slapdash that a finally block wouldn't deal with it; and it amused me to write such horrible code.
If you used a goto in production code I would fire you on the spot.
Well than you would be possibly firing Kenton Varda, Sanjay Ghemaway, Jeff Dean, and others... Oh wait, they work at Google so they don't need to work for someone as close-minded as you.
[/quote]
I love how this gets people's panties in a twist. And I love how people point out all these examples of very good programmers who've used the construct in very specific circumstances as proof that it's good, but completely ignore all of the catastrophic examples that lead to Go To Statement Considered Harmful.
I've said it before, and I'll say it again: There is absolutely no viable use case for goto in my working environment (C# 4).
I love how this gets people's panties in a twist. And I love how people point out all these examples of very good programmers who've used the construct in very specific circumstances as proof that it's good, but completely ignore all of the catastrophic examples that lead to Go To Statement Considered Harmful.
I've said it before, and I'll say it again: There is absolutely no viable use case for goto in my working environment (C# 4).
I completely agree with you about there being a distinct lack of use for goto in C# but I think the reason everyone has reacted so negatively is that your first comment makes no mention of C#, where a plethora of other methods are available. It just sounded rather flamebaitish.
Your new solution would be the one I went with as well. I have not found myself needing to use goto since my days of QBasic.
