Comment 2 for bug 487009

Revision history for this message
Michael Nelson (michael.nelson) wrote :

Here's an example diff of the above approach (BinaryPackageBuilder has-an IBuilder which to which it delegates):

http://pastebin.ubuntu.com/327583/

BinaryPackageBuilder isn't *an* IBuilder.
--------------------------------------------------

BinaryPackageBuilder implements the IBuilder interface (by delegation) which means that most of the time, it can be treated just like an IBuilder (eg. bin_builder.status or bin_builder.id all work as expected), but Storm won't be tricked... build.builder = bin_builder needs to be replaced with either bin_builder.id or bin_builder.builder.

I was already aware that call-sites would need to know what type of builder they wanted to deal with, but this means that call-sites sometimes need to know to differentiate between the BinaryPackageBuilder and the general builder that it encapsulates.

Using simple inheritance would avoid this (ie. BinaryPackageBuilder inherits Builder).

Another option that I've been thinking about while coding the above: what we're really trying to extract out is the *behaviour* of the builder. If we could define a general IBuildBehaviour which is implemented by BinaryPackageBuildBehaviour and co., then we could have the general Builder delegating IBuildBehaviour to the custom behaviour (ie. Builder has-a BuildBehaviour). The advantage here is that both call-sites *and* the general builder do not need to know about the behaviour details. For example, when the buildmaster dispatches a job, the job itself can return an appropriate build job behaviour for itself. This should ensure that extending and maintaining the different build behaviours follows good OO principles (Encapsulate what varies, favor composition over inheritance, program to interfaces - not implementation, classes should be open for extension but closed for modification, etc.)

But to do this, we need to be able to define the standard build behaviour interface (verifyBuildJob(), startBuildJob(), etc.)