The Design of Software (CLOSED)

A public forum for discussing the design of software, from the user interface to the code architecture. Now closed.

The "Design of Software" discussion group has been merged with the main Joel on Software discussion group.

The archives will remain online indefinitely.

Optimisation/Code re use/Code duplication... that sorta thing

Happy hacker that I am, I got my hands on some not so hot code and started making it nicer.  Along the way I got to thinking, sometimes the refactoring I'm doing will make something re usablee, but at the cost of an extra overhead for other users.

Being a bit slack, I can't be bothered looking it up in my library of books.

For instance, say you have 2 functions

A( int thingamy )
{
  Widgets In;
  Widgets Err;
  In.Add( thingamy );
  Err.Add( thingamy );

  DoStuff( In, null, Err );
  DoOtherStuff();
}

B( int thingamy )
{
  Widgets Out;
  Widgets Err;
  Out.Add( thingamy );
  Err.Add( thingamy );

  DoStuff( null, Out, Err );
  DoOtherStuff();
}


Now, I would normally refactor this to something like the following because DoStuff can handle an empty container passed to it.

A( int thingamy ) { C( thingamy, 1 ); }
B( int thingamy ) { C( thingamy, 2 ); }

C( int thingamy, int type )
{
  Widgets In;
  Widgets Out;
  Widgets Err;

  if( type & 1 )
    In.Add( thingamy );
  if( type & 2 )
    Out.Add( thingamy );
  Err.Add( thingamy );

  DoStuff( In, Out, Err );
  DoOtherStuff();
}

and if I need to, now I can do an in out all at once by passing a type of 3.


But whats the overhead of having the extra variables in the function, the extra function call, the tests and so on?
Would having this pattern actually be good? Is it better to spetialise in this case rather than generalise for re use?

If it where a larger part of code, I might make classes and inherit and instantiate specific classes to remove the if cases, get it compiled on in there.
Gornin Send private email
Thursday, September 28, 2006
 
 
Sweet god. On your way out, leave your refactoring license at the door. ;)

Ever heard of Cyclomatic complexity? You're refactored example is the more complicated one. In fact, if I had to edit that particular code, the first thing I would do is rewrite it to two seperate methods. Effectively undoing your refactoring.

Code reuse is good, yes, but cramming the body of 2 functions into one because they kinda, sortof do the same is asking for a headache. It needs to be a natural, estheticly pleasing fit.
Dave
Thursday, September 28, 2006
 
 
To the spelling nazi's:  Yes, I know, and I'm so ashamed.
Dave
Thursday, September 28, 2006
 
 
Why don't you pass In|Out as the second argument instead of an obsfucated constant ?
Luc Hermitte Send private email
Thursday, September 28, 2006
 
 
>  if( type & 1 )
>    In.Add( thingamy );
>  if( type & 2 )
>    Out.Add( thingamy );

"Explicit case analysis on the type of an object is usually an error,  the designer should use polymorphism in most of these cases." Heuristic #5.12, http://www.ccs.neu.edu/research/demeter/related-work/riel/heuristics2.txt

Also check out the Big Picture Issues at http://www.construx.com/survivalguide/doc/chk04.htm and/or Steve McConnell's book "Code Complete 2".
dev1
Thursday, September 28, 2006
 
 
"But whats the overhead of having the extra variables in the function, the extra function call, the tests and so on?"

The first answer to your concerns about overhead is to ignore it, and measure performance and see if it's a bottleneck. Listen to Knuth's advice on optimization.

So assuming it really is a bottleneck, and you know this because you profiled, then it will probably break down into a few areas:

1: stack allocated local variables. These don't cost you anything, if they are not initialized. If values are assigned, you will pay according to how large the data is.  Note that you might pay a *small* smount for unused uninitialized stack variables if your function does not otherwise make any use of the stack, since you will have to allocate a stack frame. That case is quite rare, though.

2: Comparisons can be expensive, particularly the first time through where all you have is static branch prediction. You can help matters slightly by using the intel static branch prediction hints (if you know which branches are most likely in the typical case). If you are doing branches to set a single variable, i'd consider using the movcc instructions, if possible -- they avoid branches for simple cases.

3: Calls are relatively cheap in and of themselves, apart from the cache issues. Call setup will vary, of course. Sometimes a call is cheaper than an inline, other times the oppsite is true. No hard and fast rule here.

 
The performance considerations, however, can be ignored. None of this is really expensive; unless you have rather unique performance requirements, you should ignore this level of detail. If you have unique performance requirements, you should still ignore it and use tools like vtune to direct your optimization efforts.

As for the pattern itself, I would favor simplicity over generalizing code. For me, i'd rather have 2 functions so I can be clear about how each is used. I'm not a big fan of XP, but really -- you probably aren't going to need it.
anonymous for this one Send private email
Thursday, September 28, 2006
 
 
Thanks for the comments.

Perhaps I cut it down too much. Or perhaps I should be more detailed in some refactoring, as suggested not bringing in uncommon portions and making if cases, but factoring out the common bits inbetween - like where I have shown DoOtherStuff which is actually many lines of code.

This example was based on a call to an external library I have no influence over, so I can't change DoStuff to use types to differentiate and such.


While I used the word type, the provided data does not include the type, it may have better been worded operation.  Polymorphism didn't seem appropriate as with the one data item you ccould perform either operation.
Gornin Send private email
Monday, October 09, 2006
 
 

This topic is archived. No further replies will be accepted.

Other recent topics Other recent topics
 
Powered by FogBugz