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

List:       kde-devel
Subject:    Re: The no goto religion
From:       Michael Scondo <michael.scondo () arcor ! de>
Date:       2007-08-03 11:09:33
Message-ID: 200708031309.34162.michael.scondo () arcor ! de
[Download RAW message or body]

On Sunday 29 July 2007 18:52, James Richard Tyrer wrote:
> A small hint from an old programmer.
>
> As you all probably know, C and C++ allow the use of the "goto" statement.
>
> When should you use a "goto".  My example:
>
> ----------------
>
> KIcon icon;
>
> for ( KIconThemeNode *themeNode = d->links.first() ; themeNode ;
>          themeNode = d->links.next() )
> {
>      icon = themeNode->theme->iconPath(file, size, KIcon::MatchExact);
>      if (icon.isValid())
>          break;
> }
>
> if ( !icon.isValid() )
> {
>      for ( KIconThemeNode *themeNode = d->links.first() ; themeNode ;
>              themeNode = d->links.next() )
>      {
>          icon = themeNode->theme->iconPath(file, size, KIcon::MatchBest);
>          if (icon.isValid())
>              break;
>      }
> }
>
> file = icon.isValid() ? icon.path : QString::null;
>

Sorry, got this thread to late, but it's too interesting to me to not hit the 
reply button.
I'd say the whole thing is about programming dogmas, use gotos or not, try 
small optimizations where possible or not.

So, here's how I'd write the snippet above:
--------------------
KIcon icon;
KIconThemeNode *themeNodeIter;

themeNodeIter = d->links.first();

while ( themeNode && !icon.isValid() ){
	icon = themeNode->theme->iconPath(file, size, KIcon::MatchExact);
	themeNodeIter = d->links.next();
}

if ( !icon.isValid() ){
	/* I'm not sure if icon.isValid is cheaper than links.first, but I guess.. */
	/* Therefore the extra if  */
	themeNodeIter =  d->links.first();
	while ( !icon.isValid() && themeNode ){ /* no Icon found yet */
		icon = themeNode->theme->iconPath(file, size, KIcon::MatchBest); 
		themeNodeIter = d->links.next();
	}
}

file = icon.isValid() ? icon.path : QString::null;
----------------------

I like to say using a break is nothing else than a goto.

I personally don't use gotos because  if I start I will not stop again.
If I think about a problem how could I do this in a while loop, I'll find a 
solution.
But if I've already gotos and breaks in my mind, I'll most possibly work with 
them.

I've had my experience with gotos in my first project.
I started programming with gwbasic around 1990 and soon wrote a small game 
collection.
At this time I had not really  an idea about programming,
so I used gotos for evereything.
It resulted in the well known spaghetti code.
Although I also wrote some subroutines without knowing about the concept, (I 
declared global vars and the linenumber where to jump back)
the whole thing was very hard to read.

So I'm not saying gotos are evil, I'm saying the way you think with gotos in 
your mind is evil.


The other dogma in this thread was about optimization.
I believe about this much in the same way about gotos.

If you've always optimizations in your mind, you'll choose the faster/smaller 
alternative.

And no, I'm not talking about a really optimization in the matter of 
benchmarking your code and looking for bottlenecks, just about writing more 
performant code where possible without extra work.

I agree that the readability must not suffer, but in the example above,  if 
I'd believe that the MatchExact iconPath will return something useful in most 
cases, I'd rewrite the line 
while ( themeNode && !icon.isValid() ) {

into
while ( !icon.isValid() && themeNode ) {

Ok, there is one comparison saved this way, however it is saved.
And again, I believe if you always have the performance in your mind, you will 
choose the more performant alternative.
The gain is nearly nothing in the example above, but there are many other 
possibilities where the performance gain would be significant.


If I reread my code:
If there was only the possiblity that I'd need this code somewhere else, I'd 
write the functions below.
But this would also be dependent on the context, if the function where the 
iconpath has to be found is already quite big, I'd also prefer the 
subroutines for a better readability.


--------------------
KIcon findIcon( const QString &name, int size, KIcon::MatchType matchtype, 
sometype &links ){
	KIcon icon;
	KIconThemeNode *themeNodeIter; 

	themeNodeIter = d->links.first();

	while ( themeNode && !icon.isValid() ){
		icon = themeNode->theme->iconPath(file, size, KIcon::MatchExact);
		themeNodeIter = d->links.next();
	}
	return( icon );
}

QString findIconPath( const QString &name, int size, sometype &links ){
	KIcon icon;
	
	icon = findIcon( name, size,  KIcon::MatchExact, links );
	if ( !icon.isValid() )
		icon = findIcon( name, size, KIcon::MatchBest, links );

	if ( icon.isValid() ){
		return icon.path;
	} else {
		return QString::null;
	}
}
----------------------


I'm not really into c at the moment, had to write most things in perl the last 
year.
Normally I'd also work with more references, just for the performance, but I'm 
quite sure even the code above won't compile, because I used somewhere perl 
syntax.

Michael
 
>> Visit http://mail.kde.org/mailman/listinfo/kde-devel#unsub to unsubscribe <<
[prev in list] [next in list] [prev in thread] [next in thread]