Is this design pattern correct (builder)

[Mike]

Hello NG,

i am just learning various Design Patterns and now i am not sure, if
this design is correct (Builder) or if i should use an other pattern.


I have various classes (here ChildA and ChildB) derived from class Base.
Now i want to create an object, but i don't want to know which class to
instantiate.

All classes have "pseudo"-methods/codes to keep it very simple.

------------------ cut here ------------------
class Base
{
public:
virtual ~Base() {}; // hack!
virtual start() = 0;
virtual stop() = 0;

// with empty methods only for default implementation
void create (const char* last, const char* first) {} ;
void create (const char* last) {} ;
};

// --------------------------------------------

class ChildA : public Base
{
public:
ChildA();
void start();
void stop();

// ok, child_a implements this create method
void create(const char* name)
{ /* do something */}
};

// --------------------------------------------
class ChildB : public Base
{
public:
ChildB();
void start();
void stop();

// ok, child_a implements the other create method
void create (const char* name1, const char* name2)
{ /* do something else */ }
};

// --------------------------------------------
class Builder
{
public:
Base* createObject(int type)
{
if (type = 1)
return new ChildA;
if (type = 2)
return new ChildB;

// exception handling
...
...
}
} ;

// --------------------------------------------

int main()
{
Base base = Builder::createObject(2); // ChildB
base->create("Doe", "Johnny");
base->start();

/* do the rest */
...
...
}

------------------ cut here ------------------



Thanks a lot for your answers
Mike

 

[Luke Meyers]

Hello NG,

Hello, Mike.

i am just learning various Design Patterns and now i am not sure, if
this design is correct (Builder) or if i should use an other pattern.

Do you want a critique on your choice of this pattern, your
implementation, or both?

I have various classes (here ChildA and ChildB) derived from class Base.
Now i want to create an object, but i don't want to know which class to
instantiate.

I don't have my DP book handy, but this sounds a lot more like Abstract
Factory or Factory Method to me, depending on the context. Builder is
about generalizing the construction of complex (as in having many
parts) objects which are different but have a similar set of
construction steps.

All classes have "pseudo"-methods/codes to keep it very simple.

Might as well take out all the start() and stop() business then, too.

------------------ cut here ------------------
class Base
{
public:
virtual ~Base() {}; // hack!

How is this a hack?

// with empty methods only for default implementation
void create (const char* last, const char* first) {} ;
void create (const char* last) {} ;

These should almost certainly be virtual, considering that you're
talking about a "default implementation," and have written member
functions with the same signatures in derived classes. Creational
design patterns build heavily on the basic concepts and techniques of
polymorphism -- trying to use the one without a thorough understanding
of the other is unlikely to be fruitful.

class Builder
{
public:
Base* createObject(int type)
{
if (type = 1)
return new ChildA;
if (type = 2)
return new ChildB;

Type codes are pretty ugly. At the very least, use an enumerated type
here to reduce worries about out-of-range values. Better, though,
would be to change the design. I don't believe GoF describes any
creational patterns involving passing a description of the type as a
parameter -- they mostly rely on polymorphism instead.

int main()
{
Base base = Builder::createObject(2); // ChildB
base->create("Doe", "Johnny");
base->start();

Seems like either you've chosen a rather poorly-named member function
("create") for your example, or you're misunderstanding the
responsibilities involved here. What is being created by create()?
You've already created a ChildA or ChildB, and create() doesn't return
anything. The creational part is going on in createObject(). Having
something else called create() just confuses the example, at best.

So, in sum... this doesn't look much like the Builder pattern; it looks
more like a somewhat ill-informed implementation of some other pattern.
I suggest you read up on all of the creational patterns, especially
Abstract Factory and Factory Method, and think carefully about which
one solves the problem you're facing. Use the UML diagrams as a quick
way to check whether your implementation even has the right number of
entities to represent the concepts used by the pattern.

Luke

 

[Stuart Golodetz]

Hello, Mike.


Do you want a critique on your choice of this pattern, your
implementation, or both?



Type codes are pretty ugly. At the very least, use an enumerated type
here to reduce worries about out-of-range values. Better, though,
would be to change the design. I don't believe GoF describes any
creational patterns involving passing a description of the type as a
parameter -- they mostly rely on polymorphism instead.

This seems to always return a pointer to a ChildA as well. Note that if(type
= 1) sets type to 1, checks whether it's non-zero (implicitly), finds that
it is and then returns a pointer to a new ChildA. Presumably the OP meant
if(type == 1) etc. But what you said about the whole type code thing in
general being pretty dodgy still stands (changing = to == here is akin to
putting a sticking plaster over a gaping wound - it doesn't really fix the
underlying issue, even if it makes you feel like you're doing something
constructive).

Cheers
Stu

<snip other stuff>