Mailinglist Archive: zypp-devel (115 mails)

< Previous Next >
[zypp-devel] class functions!!!! Polluted APIs
  • From: Duncan Mac-Vicar Prett <dmacvicar@xxxxxxx>
  • Date: Fri, 6 Jul 2007 11:37:02 +0200
  • Message-id: <200707061137.03038.dmacvicar@xxxxxxx>

Some notes on adding functons to classes.

Schubi ran nto a problem today where the testsuite pass on his system but 
failed into the autobuild environment, because target was not able to create 
the directories in /var/lib/zypp (non-root), while in schubi's machine this 
directories already existed.

The reason was, in a piece of code of the test suite, the target was 
initialized, to use the getResolvablesToInsDel function.

Accessing the target in the testsuite is sick. Because the installed 
resolvables come from a xml file, and not on the system target, you don't 
want to make the testsuite results to depend on the host system installed 
programs. So as long you access the target and you should't you should realze 
the function you are calling it is in the wrong place.

Lets check the function.

void
TargetImpl::getResolvablesToInsDel ( const ResPool pool_r,
                                   TargetImpl::PoolItemList & dellist_r,
                                   TargetImpl::PoolItemList & instlist_r,
                                   TargetImpl::PoolItemList & srclist_r ) 
const
{
   pool::GetResolvablesToInsDel collect( pool_r );
    MIL << "GetResolvablesToInsDel: " << endl << collect << endl;
    dellist_r.swap( collect._toDelete );
    instlist_r.swap( collect._toInstall );
    srclist_r.swap( collect._toSrcinstall );
}

So, the function does not access any TargetImpl data, so _why_ is it in the 
class? Unless you have workflow classes (or facades) you don't need to have 
the function in the class.

Look this function. It operates on the pool data, and it is const, so you can 
safely have it in the pool, without touching its contents. But having it in 
Target (with no reason ) means you have to init the target before using it, 
and lot of things happen during target init.

The worst part, the comment on the functon docs:

/** JUST FOR TESTSUITE */
/** Sort according to prereqs and media numbers */

If it was only needed by the testsuite, why not just using 
zypp/pool/GetResolvablesToInsDel.h directly, after all the other function 
just copies the lists from the collected functor, why not doing that in the 
testsuite itself?

I removed the functon from Target interfaces, Schubi modified the testsuite to 
use GetResolvablesToInsDel.h directly.

So, if you can't find a place where to put a function it, just make it a 
global function in a namespace, you will at least avoid initializing an 
object just to use that functon!.

-- 
Duncan Mac-Vicar Prett  
Novell :: SUSE R&D, Maxfeldstr. 5, 90409 Nürnberg
GF: Markus Rex, HRB 16746 (AG Nürnberg)
--
To unsubscribe, e-mail: zypp-devel+unsubscribe@xxxxxxxxxxxx
For additional commands, e-mail: zypp-devel+help@xxxxxxxxxxxx

< Previous Next >
This Thread
  • No further messages