[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-commits
Subject:    Re: kdelibs/khtml
From:       Tobias Anton <tobias.anton () esc-electronics ! de>
Date:       2005-04-08 12:21:17
Message-ID: 200504081421.17915.tobias.anton () esc-electronics ! de
[Download RAW message or body]

Am Donnerstag, 7. April 2005 19:24 schrieb Lubos Lunak:
> > What about a
> > 	"do ... while (!necessary);"
> > or even a
> > 	"do { ... continue; } while (false);"
> > loop?
>
>  Just curious, what benefits should this exactly bring, other than
> affecting cvs annotate for 100+ lines and making Dijkstra happy?

Readability. Every single backward goto can be expressed by a loop. A loop is 
easier to read because you don't have to look up any label's position to 
figure out whether a "continue" or "break" results in a backward or forward 
jump.

Comparing your backward goto to a loop, I don't see any advantage of the 
backward goto. The main disadvantage of the backward goto: The exit condition 
is not clear, whereas a loop would make it clear. Finally, introducing a loop 
makes no difference in the number of lines at all:

-again:
+    do {

-                // don't return until necessary!
-                goto again;

+    } while (state == decompressStarted);

As you can see, this patch replaces 2 lines of code, not 100+, so your 
argument is wrong. If you care about code indentation, things are different, 
admitted, but I care for cvs annotate less than for the code itself. A flaw 
of our SCM should not affect the developed software, otherwise it's a bad 
SCM.

Cheers
-- Tobias


["temp.diff" (text/x-diff)]

Index: misc/loader_jpeg.cpp
===================================================================
RCS file: /home/kde/kdelibs/khtml/misc/loader_jpeg.cpp,v
retrieving revision 1.29
diff -u -3 -p -r1.29 loader_jpeg.cpp
--- misc/loader_jpeg.cpp	5 Apr 2005 12:21:27 -0000	1.29
+++ misc/loader_jpeg.cpp	8 Apr 2005 12:16:16 -0000
@@ -375,7 +375,7 @@ int KJPEGFormat::decode(QImage& image, Q
         }
     }
 
-again:
+    do {
 
     if(state == decompressStarted) {
         state =  (!jsrc.final_pass && jsrc.decoder_timestamp.elapsed() < max_consumingtime)
@@ -482,8 +482,6 @@ again:
 #endif
                 jsrc.decoder_timestamp.restart();
                 state = decompressStarted;
-                // don't return until necessary!
-                goto again;
             }
         }
 
@@ -507,6 +505,7 @@ again:
             return 0;
         }
     }
+    } while (state == decompressStarted);
 
 #ifdef BUFFER_DEBUG
     qDebug("valid_buffer_len is now %d", jsrc.valid_buffer_len);


[prev in list] [next in list] [prev in thread] [next in thread]