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 > |