Abstract Factory or Factory Method pattern question....

[Medi Montaseri]

Hi,

Given a collection of similar but not exact entities (or products)
Toyota, Ford, Buick, etc; I am contemplating using the Abstraction pattern
to provide a common interface to these products. So I shall have an
Abstract Base called 'Car' implemented by Toyota, Ford, and Buick.

Further I'd like to enable to client to say

Car *factory;
Car *mycar = factory->make ( "Ford" );

However note how 'mycar' is of type 'Car' and although make() will
return (new Ford ), an up-cast will take place and what client now has
is an instance of Car (abstract) and not Ford (concrete).

True that with Virtual Methods, the appropriate method will be called,
but I now have a data-member problem as the newly created and returned
object will only have access to Base data members. Again I don't want the
client to be aware of what type of 'Car' he/she will be using, ie I don't want
(if possible) to force the client to say

Ford *mycar = factory->make ( "Ford" )

Because client is now becomming aware of 'Car' vs 'Ford'.

One answer would be to have the Base have all the data members (superset)
of any Car-type. But now Ford has to know what is needed for it and have code
that guard against under usage or over usage. ie Ford would say, I don't need this
data member, why is it here...

Another answer would be, sorry can't do. Your client has to provide the
correct lval type during the object creation to reach the exact object.

If the client is to be aware, then I'm loosing part of Abstraction , ie who cares
wether its a Toyota or Ford, to my client its just a Car.

By the way I know my people who actually think that way about cars.... :-)

[Buster Copley]

Attila Feher wrote:

Medi Montaseri wrote:

Hi,

Given a collection of similar but not exact entities (or products)
Toyota, Ford, Buick, etc; I am contemplating using the Abstraction
pattern
to provide a common interface to these products. So I shall have an
Abstract Base called 'Car' implemented by Toyota, Ford, and Buick.

Further I'd like to enable to client to say

Car *factory;
Car *mycar = factory->make ( "Ford" );
Really? I don't think that will work. Dereferencing an uninitialized
pointer leads to undefined behaviour.
However note how 'mycar' is of type 'Car' and although make() will
return (new Ford ), an up-cast will take place and what client now has
is an instance of Car (abstract) and not Ford (concrete).
No, 'mycar' is of type 'Car *'. The pointer the client gets points to an
instance of Ford. There is no such thing as an instance of Car.
True that with Virtual Methods, the appropriate method will be called,
but I now have a data-member problem as the newly created and returned
object will only have access to Base data members.

The object has access to all its members. It's the client who only has
access to public members defined in the base class, Car, and that's the
way it's supposed to be. Since the caller isn't supposed to care what
kind of car he's got, he can only ask the car to do what any Car can do.
If he wants to do model-specific stuff, he first has to find out what
car he's driving, and once he's done that he can get a derived class
pointer.

By the way, it's unusual for an abstract base class to have any public
data members. The important thing is what information you can ask for
and what actions you can request. How the information is stored is as
much an implementation detail as how the actions are to be performed.
[SNIP]

Could you elaborate what is this "data member problem"? I cannot see it.
It is a frequently used method to create an absolutely pure abstract class
(no data members), provide a factory (even such, which can be extended by
the concrete classes registering themselves), possibly provide some
handle-body class to manage the returned pointer and tada, it is there,
working. All you need (IMHO) to overcome any inconvenience regarding
different data members is to have a virtual clone member function to be able
to copy a Car - which is Ford - and still have a Ford, not just a dataless
Car.

BTW, IMHO this approach to describe cars is not the best design. I would go
for a solution where the type of the car is just an attribute. AFAIS the
whole Car concept is rather a concept of data (describing the car) rather
than a hierarchy. Also if I think about the maintenance it sounds really
bad that with every new car the programmer has to write a new class.
Somehow it does not sound right.

Think on. I'd rather define a whole new car in one place than go round
every part of my program that depends on the type of car I'm using and
add an extra 'case' statement. I think of polymorphism as a way to
transform code full of switch statements into something more readable
and modular.

Regards,
Buster.

[Attila Feher]

Buster Copley wrote:

Attila Feher wrote:

[SNIP]

Car *factory;
Car *mycar = factory->make ( "Ford" );
Really? I don't think that will work. Dereferencing an uninitialized
pointer leads to undefined behaviour.

Yes, and no. :-) (I have missed that one.) In real life static functions
(like make) do not make any use of the pointer itself, they are resolved at
compile time. I have no idea what the standard says about this specific
case. Here the pointer is not really dereferenced - since factory functions
are static.

However note how 'mycar' is of type 'Car' and although make() will
return (new Ford ), an up-cast will take place and what client now
has
is an instance of Car (abstract) and not Ford (concrete).
No, 'mycar' is of type 'Car *'. The pointer the client gets points to
an instance of Ford. There is no such thing as an instance of Car.

IMHO that is what he meant. But the correction is absolutely valid. :-)

[SNIP]
Could you elaborate what is this "data member problem"? I cannot
see it. It is a frequently used method to create an absolutely pure
abstract class (no data members), provide a factory (even such,
which can be extended by the concrete classes registering
themselves), possibly provide some handle-body class to manage the
returned pointer and tada, it is there, working. All you need
(IMHO) to overcome any inconvenience regarding different data
members is to have a virtual clone member function to be able to
copy a Car - which is Ford - and still have a Ford, not just a
dataless Car.

BTW, IMHO this approach to describe cars is not the best design. I
would go for a solution where the type of the car is just an
attribute. AFAIS the whole Car concept is rather a concept of data
(describing the car) rather than a hierarchy. Also if I think about
the maintenance it sounds really bad that with every new car the
programmer has to write a new class. Somehow it does not sound right.

Think on. I'd rather define a whole new car in one place than go round
every part of my program that depends on the type of car I'm using and
add an extra 'case' statement. I think of polymorphism as a way to
transform code full of switch statements into something more readable
and modular.

To whom did you write that? You have replied to me, and my suggestion did
not contain switch statements at all.
--
Attila aka WW

[Buster Copley]

Attila Feher wrote:

Buster Copley wrote:

Attila Feher wrote:
[SNIP]

Car *factory;
Car *mycar = factory->make ( "Ford" );

Really? I don't think that will work. Dereferencing an uninitialized
pointer leads to undefined behaviour.

Yes, and no. :-) (I have missed that one.) In real life static functions
(like make) do not make any use of the pointer itself, they are resolved at
compile time. I have no idea what the standard says about this specific
case. Here the pointer is not really dereferenced - since factory functions
are static.

Indeed, but that's not the syntax to call a static member function.
[snip]

BTW, IMHO this approach to describe cars is not the best design. I
would go for a solution where the type of the car is just an
attribute. AFAIS the whole Car concept is rather a concept of data
(describing the car) rather than a hierarchy. Also if I think about
the maintenance it sounds really bad that with every new car the
programmer has to write a new class. Somehow it does not sound right.

Think on. I'd rather define a whole new car in one place than go round
every part of my program that depends on the type of car I'm using and
add an extra 'case' statement. I think of polymorphism as a way to
transform code full of switch statements into something more readable
and modular.

To whom did you write that? You have replied to me, and my suggestion did
not contain switch statements at all.

To you. Your suggestion didn't contain any C++ at all. Whether it's
else-if ladders, switch statements, associative arrays or whatever,
somehow you have to decide what to do based on an "attribute" (private
data member?) of your object.
--
Attila aka WW

[Attila Feher]

Buster Copley wrote:

Attila Feher wrote: [SNIP]

BTW, IMHO this approach to describe cars is not the best design. I
would go for a solution where the type of the car is just an
attribute. AFAIS the whole Car concept is rather a concept of data
(describing the car) rather than a hierarchy. Also if I think
about
the maintenance it sounds really bad that with every new car the
programmer has to write a new class. Somehow it does not sound
right.

[SNIP] To you. Your suggestion didn't contain any C++ at all. Whether it's
else-if ladders, switch statements, associative arrays or whatever,
somehow you have to decide what to do based on an "attribute" (private
data member?) of your object.

Please read what I wrote. I said in there that I do not think that a class
hierarchy is a good idea. Car seems to consist of nothing else, but data.
Meaning: there is a Car class having all possible data for a car (number of
doors, colors etc.). If you would ever need some algorithm difference you
can use the Strategy pattern.

--
Attila aka WW

[Medi Montaseri]

"Attila Feher" <at**********@lmf.ericsson.se> wrote in message news:<bi**********@newstree.wise.edt.ericsson.se>. ..

Medi Montaseri wrote:

Hi,

Given a collection of similar but not exact entities (or products)
Toyota, Ford, Buick, etc; I am contemplating using the Abstraction
pattern
to provide a common interface to these products. So I shall have an
Abstract Base called 'Car' implemented by Toyota, Ford, and Buick.

Further I'd like to enable to client to say

Car *factory;
Car *mycar = factory->make ( "Ford" );

However note how 'mycar' is of type 'Car' and although make() will
return (new Ford ), an up-cast will take place and what client now has
is an instance of Car (abstract) and not Ford (concrete).

True that with Virtual Methods, the appropriate method will be called,
but I now have a data-member problem as the newly created and returned
object will only have access to Base data members. [SNIP]

Could you elaborate what is this "data member problem"? I cannot see it.
It is a frequently used method to create an absolutely pure abstract class
(no data members), provide a factory (even such, which can be extended by
the concrete classes registering themselves), possibly provide some
handle-body class to manage the returned pointer and tada, it is there,
working. All you need (IMHO) to overcome any inconvenience regarding
different data members is to have a virtual clone member function to be able
to copy a Car - which is Ford - and still have a Ford, not just a dataless
Car.

I'm not following the "Virtual Cloen member function"...
if you mean a factory method, then Car::Make() is that...

BTW, IMHO this approach to describe cars is not the best design. I would go
for a solution where the type of the car is just an attribute. AFAIS the
whole Car concept is rather a concept of data (describing the car) rather
than a hierarchy. Also if I think about the maintenance it sounds really
bad that with every new car the programmer has to write a new class.
Somehow it does not sound right.

So what I want to achieve is to have a common interface so that I as the
author of that interface can mandate that N number of methods should be
implemented. These services (or methods) fall in two (or more) categories;

cat-1) Methods that are specific to a particular car.
Implemented by concrete classes.
cat-2) Methods that are common (reusability).
Implemented in the abstract base class (Car).

My data-member problem surfaces at compile time where a "common" method
defined in base is referencing a data-member. A Data-Less Base class
as you recommended would have problem with this.

This is also due to the desirable feature that client should be able
to say

Car *mycar = factory->Make( "Ford" );
....
mycar->getColor();

At compile time since mycar is an object of type Car will try to look for
Car::_color and not Ford::_color.

Hence having a data-less base class will yield compile time error...

[Attila Feher]

Medi Montaseri wrote:

"Attila Feher" <at**********@lmf.ericsson.se> wrote in message [SNIP] I'm not following the "Virtual Cloen member function"...
if you mean a factory method, then Car::Make() is that...
I meant clone(). If you have a pointer to a Car object (which might as well
point to a Ford) you will not be able to copy that Ford unless you ask it to
copy itself. Since you have no idea what it is. That clone is a simple
virtual function returning a (poiter to a) newly allocated copy of the
object itself. {return new Ford(*this);} Since it is virtual a Ford will
return a Ford, a Toyota a Toyota etc.

[SNIP] So what I want to achieve is to have a common interface so that I as
the
author of that interface can mandate that N number of methods should
be
implemented. These services (or methods) fall in two (or more)
categories;

cat-1) Methods that are specific to a particular car.
Implemented by concrete classes.
cat-2) Methods that are common (reusability).
Implemented in the abstract base class (Car).

My data-member problem surfaces at compile time where a "common"
method
defined in base is referencing a data-member. A Data-Less Base class
as you recommended would have problem with this.

This is also due to the desirable feature that client should be able
to say

Car *mycar = factory->Make( "Ford" );
...
mycar->getColor();

At compile time since mycar is an object of type Car will try to look
for
Car::_color and not Ford::_color.

Hence having a data-less base class will yield compile time error...

I understood that. If you look closely to what I have suggested was that
look at your design again, since I do not believe that Ford, Toyota etc.
should ever be separate classes. I think those belong to an std::string
member called manufacturer_. I also think that this Car object is rather a
collection of data then behavior, so I do not see any need for a class
hierarchy (inheritance). I might be wrong of course.

--
Attila aka WW

[Medi Montaseri]

Buster Copley <bu****@none.com> wrote in message news:<bi**********@newsg4.svr.pol.co.uk>...

Attila Feher wrote:

Medi Montaseri wrote:

Hi,

Given a collection of similar but not exact entities (or products)
Toyota, Ford, Buick, etc; I am contemplating using the Abstraction
pattern
to provide a common interface to these products. So I shall have an
Abstract Base called 'Car' implemented by Toyota, Ford, and Buick.

Further I'd like to enable to client to say

Car *factory;
Car *mycar = factory->make ( "Ford" );
Really? I don't think that will work. Dereferencing an uninitialized
pointer leads to undefined behaviour.
Yes I think this is allowed for abstract classes. ie you can invoke the
method via pointer and references. But not an instance of it, as in

Car factoryr; factory.make( "Ford" ); // this will not work
Car *factory; factory->make( "Ford" ); // this will work
Car &factory; factory.make( "Ford" ) ; // this will work

However note how 'mycar' is of type 'Car' and although make() will
return (new Ford ), an up-cast will take place and what client now has
is an instance of Car (abstract) and not Ford (concrete).
No, 'mycar' is of type 'Car *'. The pointer the client gets points to an
instance of Ford. There is no such thing as an instance of Car.

Purely speaking you are correct, there is no instance. mycar is of type "Car *".

True that with Virtual Methods, the appropriate method will be called,
but I now have a data-member problem as the newly created and returned
object will only have access to Base data members.

If he wants to do model-specific stuff, he first has to find out what
car he's driving, and once he's done that he can get a derived class
pointer.
Exactly, so the question is ... can we even hide that so that all common
features of Car and specific features of a particular car are provided to
the client from one single object. Call it Single Point of Service (or Interface).

By the way, it's unusual for an abstract base class to have any public
data members. The important thing is what information you can ask for
and what actions you can request. How the information is stored is as
much an implementation detail as how the actions are to be performed.
Yes but if our model is house all common methods in the base and specific
methods in their respective concrete classes, then at compile time, a common
method (which is defined in base) will try to reach a data member that thinks
should be available in the class where method is defined. and of course a
dataless base will not have any methods.....

So perhaps the issue is how I'm trying to hit two birds with this approach;
I want reusability (by having common methods in base) and I also want
Interfacing. Maybe this is not a good mix...


Regards,
Buster.

[Buster Copley]

Medi Montaseri wrote:

Yes I think this is allowed for abstract classes. ie you can invoke the
method via pointer and references. But not an instance of it, as in

Car factoryr; factory.make( "Ford" ); // this will not work
Car *factory; factory->make( "Ford" ); // this will work
Car &factory; factory.make( "Ford" ) ; // this will work
I think you're correct. (make () is not a virtual function, right?)

If he wants to do model-specific stuff, he first has to find out what
car he's driving, and once he's done that he can get a derived class
pointer.

Exactly, so the question is ... can we even hide that so that all common
features of Car and specific features of a particular car are provided to
the client from one single object. Call it Single Point of Service (or Interface).

I'm not sure what you mean. Doesn't a derived class pointer give you
access to all common features of car and specific features of a
particular car?

If you provide a way to access all possible features of any specific car
from a base class pointer, then you have constrained what features are
possible in future cars. As my friend Atilla has said, it doesn't seem
unreasonable to impose such constraints ["there is a Car class having
all possible data for a car (number of doors, colors etc.)"]. But I
don't think you are coming from the same angle, because there would be
no "data member problem" then.

May I say that it's not correct that "the newly created and returned
object will only have access to Base class data members". The object has
access to derived class data members through its virtual functions. Does
that help you at all? (Don't take offense if I'm pointing out the
obvious - I don't know your level, that's all.)

By the way, it's unusual for an abstract base class to have any public
data members. The important thing is what information you can ask for
and what actions you can request. How the information is stored is as
much an implementation detail as how the actions are to be performed.

Yes but if our model is house all common methods in the base and specific
methods in their respective concrete classes, then at compile time, a common
method (which is defined in base) will try to reach a data member that thinks
should be available in the class where method is defined. and of course a
dataless base will not have any methods.....

I find this hard to follow. If a base class method depends on data
members, that data should be in the base class (or the method should be
in the derived class). As a compromise, perhaps you could solve your
problem by providing virtual helper functions for your base class
method, so that the information about derived data members is
'translated' to a form useful to the base class method in the correct
way for each derived class.

Tell me if you think I've missed the point.
So perhaps the issue is how I'm trying to hit two birds with this approach;
I want reusability (by having common methods in base) and I also want
Interfacing. Maybe this is not a good mix...

Regards,
Buster.

[Big Brian]

> Perhaps I should include some code, maybe I'm failing to express my

problems...I'll present a Car.cc, Sedan.cc and Pickup.cc. Which is
a small change from Car, Ford, Toyota...but I wanted to show how
some cars might have different functionalities such as a Pickup truck
has openTailGate() where Sedan has openRearDoor()...

// -------------------- here is Car.h -----------------
#ifndef _CAR_H_
#define _CAR_H_
#include <iostream>
#include <string>

using namespace std;

class Car
{
public:
// a factory method
Car * make(const string& carType = "" );

// a mandatory method for derived classes to implement
virtual void Drive() = 0;

// a common method without any data access...works
void greetDriver() { cout << "Hello driver." << endl ; }

// a common method with instance data access...don't work
void getColor() { cout << "Color=" << _color << endl; }
This is a compiler error. There is no _color defined in this class.

// car model specific methods
//virtual void openRearDoor();
//virtual void openTailGate();
};

#endif
// --------------------- here is Sedan.h ---------------------
#ifndef _SEDAN_H_
#define _SEDAN_H_

#include <string>
#include "Car.h"

using namespace std;

class Sedan: public Car
{
public:
Sedan();
virtual ~Sedan() {};
void Drive();
void openRearDoor();

private:
string _color;
string _RearDoorState;
};

#endif
// -------------------- here is Pickup.h -----------------
#ifndef _PICKUP_H_
#define _PICKUP_H_

#include <string>
#include "Car.h"

using namespace std;

class Pickup: public Car
{
public:
Pickup();
virtual ~Pickup() {};
void openTailGate();
void Drive();

private:
string _color;
string _TailGateState;
};
Yuck, why are you storing the state in a string? This is inefficient.

#endif
// ------------------- here is Car.cpp --------------
#include <iostream>
#include "Car.h"
#include "Sedan.h"
#include "Pickup.h"

using namespace std;

// ---------------- make() ------------
Car* Car::make(const string& carType /* = "" */ )
{
if ( carType == "Sedan" )
{
return( new Sedan() );
}
else if ( carType == "Pickup" )
{
return( new Pickup() );
}
else
{
cerr << "Error: You need to specify a car type" << endl;
return(NULL);
}
}
// ----------------------
// ------------------------ here is Sedan.cpp -------------
#include <iostream>
#include "Sedan.h"

using namespace std;

// ------------------------ Sedan() -----------------
Sedan::Sedan(): _color("White"), _RearDoorState("closed")
{
}
// --------------------- openRearDoor() -----------
void Sedan::openRearDoor()
{
_RearDoorState = "open";
cout << "Ok...Rear door is now open." << endl;
}
// --------------------- Drive() -----------
void Sedan::Drive()
{
cout << "Ok...I'm driving." << endl;
}
// ----------------
// ----------------- here is Pickup.cpp ----------------
#include <iostream>
#include "Pickup.h"

using namespace std;

// ------------------------ Pickup() -----------------
Pickup::Pickup(): _color("Black"), _TailGateState("closed")
{
}
// --------------------- openTailGate() -------------
void Pickup::openTailGate()
{
_TailGateState = "open";
cout << "Ok...Tail gate is now open." << endl;
}
// --------------------- Drive() -------------
void Pickup::Drive()
{
cout << "Ok...i'm driving." << endl;
}
// ---------------------
// ----------------------- here is the client foo.cpp -------------
#include <iostream>
#include "Car.h"
#include "Sedan.h"
using namespace std;

int main (int argc, char **argv)
{
cout << "Starting.... " << endl;

Car *factory ;
Car *mySedan = factory->make( "Sedan" );
Car *myPickup = factory->make( "Pickup" );

mySedan->greetDriver() ;
//mySedan->openRearDoor();

myPickup->greetDriver() ;
//myPickup->openTailGate() ;

delete ( mySedan );
delete ( myPickup );
}
// ----------------------- here is the compiler error --------
medi@medi:~/cpp/car> make foo
g++ -c foo.cpp Car.cpp Sedan.cpp Pickup.cpp
In file included from foo.cpp:2:
Car.h: In member function `void Car::getColor()':
Car.h:21: `_color' undeclared (first use this function)
Car.h:21: (Each undeclared identifier is reported only once for each function
it appears in.)
In file included from Car.cpp:2:
Car.h: In member function `void Car::getColor()':
Car.h:21: `_color' undeclared (first use this function)
Car.h:21: (Each undeclared identifier is reported only once for each function
it appears in.)
In file included from Sedan.h:5,
from Sedan.cpp:2:
Car.h: In member function `void Car::getColor()':
Car.h:21: `_color' undeclared (first use this function)
Car.h:21: (Each undeclared identifier is reported only once for each function
it appears in.)
In file included from Pickup.h:5,
from Pickup.cpp:2:
Car.h: In member function `void Car::getColor()':
Car.h:21: `_color' undeclared (first use this function)
Car.h:21: (Each undeclared identifier is reported only once for each function
it appears in.)
make: *** [foo] Error 1
medi@medi:~/cpp/car>
Note how Car::getColor() is failing to access Sedan::_color data member.
showing that
Yes, that's right because _color is a member of Sedan not Car. Car
cannot access Sedan's data member directly because Sedan is derived
from Car and not the other way around. ( but you could use a virtual
function that's overridden in Sedan to return Sedan::_color )
mySedan->getColor() is sending a message to object of type Car and not
a Sedan. At least at compile time...

No, you have a compiler error ( which appears to be from your
mis-understanding of C++ ). Also, you really aren't "sending a
message", your just dereferencing a pointer to a class ( who's type
can be determined at run time ) and calling a member function.

[Kevin Goodsell]

(Note: Excuse my replying to a reply rather than the actual post. It has
not arrived on my server.)

Perhaps I should include some code, maybe I'm failing to express my
problems...I'll present a Car.cc, Sedan.cc and Pickup.cc. Which is
a small change from Car, Ford, Toyota...but I wanted to show how
some cars might have different functionalities such as a Pickup truck
has openTailGate() where Sedan has openRearDoor()...

// -------------------- here is Car.h -----------------
#ifndef _CAR_H_
#define _CAR_H_
This is illegal. Identifiers beginning with an underscore followed by an
uppercase letter are reserved for the implementation, and you may not
use them.

Always assume that the compiler has #defined identifiers like this to
something like system("format c: /u /y") or something equally terrible.
You'll be lucky if it doesn't compile.

As a general rule, never begin an identifier with an underscore.
Sometimes it's legal, but it's much easier to just avoid it altogether
than to remember the exact rules. Also, you may not use an identifier
that contains two adjacent underscores.
#include <iostream>
#include <string>

using namespace std;
Please don't do this in a header file. You are polluting the global
namespace for everyone who attempts to use this header. If you want to
do it in your own .cpp files, that's one thing. But in a header that you
expect to be useful, you should not do this.

class Car
{
public:
// a factory method
Car * make(const string& carType = "" );

// a mandatory method for derived classes to implement
virtual void Drive() = 0;

// a common method without any data access...works
void greetDriver() { cout << "Hello driver." << endl ; }

// a common method with instance data access...don't work
void getColor() { cout << "Color=" << _color << endl; }

This is a compiler error. There is no _color defined in this class.

// car model specific methods
//virtual void openRearDoor();
//virtual void openTailGate();
};

#endif
// --------------------- here is Sedan.h ---------------------
#ifndef _SEDAN_H_
#define _SEDAN_H_
Still illegal.

#include <string>
#include "Car.h"

using namespace std;
Still a very bad idea.

class Sedan: public Car
{
public:
Sedan();
virtual ~Sedan() {};
Semi-colons don't go after functions.
void Drive();
void openRearDoor();

private:
string _color;
string _RearDoorState;
Also illegal.
};

#endif
// -------------------- here is Pickup.h -----------------
#ifndef _PICKUP_H_
#define _PICKUP_H_
Illegal.

#include <string>
#include "Car.h"

using namespace std;
Brain-damaged.

class Pickup: public Car
{
public:
Pickup();
virtual ~Pickup() {};
Drop the semi-colon.
void openTailGate();
void Drive();

private:
string _color;
string _TailGateState;
Illegal.
};

Yuck, why are you storing the state in a string? This is inefficient.

#endif
// ------------------- here is Car.cpp --------------
#include <iostream>
#include "Car.h"
#include "Sedan.h"
#include "Pickup.h"

using namespace std;

// ---------------- make() ------------
Car* Car::make(const string& carType /* = "" */ )
{
if ( carType == "Sedan" )
{
return( new Sedan() );
}
else if ( carType == "Pickup" )
{
return( new Pickup() );
}
else
{
cerr << "Error: You need to specify a car type" << endl;
return(NULL);
}
}
// ----------------------
// ------------------------ here is Sedan.cpp -------------
#include <iostream>
#include "Sedan.h"

using namespace std;

// ------------------------ Sedan() -----------------
Sedan::Sedan(): _color("White"), _RearDoorState("closed")
Illegal.
{
}
// --------------------- openRearDoor() -----------
void Sedan::openRearDoor()
{
_RearDoorState = "open";
Illegal.
cout << "Ok...Rear door is now open." << endl;
}
// --------------------- Drive() -----------
void Sedan::Drive()
{
cout << "Ok...I'm driving." << endl;
}
// ----------------
// ----------------- here is Pickup.cpp ----------------
#include <iostream>
#include "Pickup.h"

using namespace std;

// ------------------------ Pickup() -----------------
Pickup::Pickup(): _color("Black"), _TailGateState("closed")
Illegal.
{
}
// --------------------- openTailGate() -------------
void Pickup::openTailGate()
{
_TailGateState = "open";
Illegal.
cout << "Ok...Tail gate is now open." << endl;
}
// --------------------- Drive() -------------
void Pickup::Drive()
{
cout << "Ok...i'm driving." << endl;
}
// ---------------------
// ----------------------- here is the client foo.cpp -------------
#include <iostream>
#include "Car.h"
#include "Sedan.h"
using namespace std;

int main (int argc, char **argv)
{
cout << "Starting.... " << endl;

Car *factory ;
Car *mySedan = factory->make( "Sedan" );
Here you dereference an uninitialized pointer. That is very, very bad.
Car *myPickup = factory->make( "Pickup" );

mySedan->greetDriver() ;
//mySedan->openRearDoor();

myPickup->greetDriver() ;
//myPickup->openTailGate() ;

delete ( mySedan );
delete ( myPickup );
}
// ----------------------- here is the compiler error --------
medi@medi:~/cpp/car> make foo
g++ -c foo.cpp Car.cpp Sedan.cpp Pickup.cpp
In file included from foo.cpp:2:
Car.h: In member function `void Car::getColor()':
Car.h:21: `_color' undeclared (first use this function)
Car.h:21: (Each undeclared identifier is reported only once for each function
it appears in.)
In file included from Car.cpp:2:
Car.h: In member function `void Car::getColor()':
Car.h:21: `_color' undeclared (first use this function)
Car.h:21: (Each undeclared identifier is reported only once for each function
it appears in.)
In file included from Sedan.h:5,
from Sedan.cpp:2:
Car.h: In member function `void Car::getColor()':
Car.h:21: `_color' undeclared (first use this function)
Car.h:21: (Each undeclared identifier is reported only once for each function
it appears in.)
In file included from Pickup.h:5,
from Pickup.cpp:2:
Car.h: In member function `void Car::getColor()':
Car.h:21: `_color' undeclared (first use this function)
Car.h:21: (Each undeclared identifier is reported only once for each function
it appears in.)
make: *** [foo] Error 1
medi@medi:~/cpp/car>

I didn't look at the code all that closely, but I'd say the message to
which I'm replying identified the main problem that the compiler is
complaining about.

-Kevin
--
My email address is valid, but changes periodically.
To contact me please use the address from a recent posting.

[Medi Montaseri]

Thank you very much Kevin for your comments....

The idea behind posting this sample code was not to illustrate good
programming
styles or practices, but to express my problem statement via simple
program.
I appreciate your feedback and will include them in the production
quality code...

Back to the higher level problem....I'm trying to understand the
Abstract Factory
or Factory Method pattern as discussed in various pattern oriented
books...

My current understanding is that one would use the Abstract pattern to
1) Provide a single point of contact to the client
2) Establish an outline or architecture of what will (or must) be
provided for
an interface via pure virtual functions.
I was also trying to stretch the Abstract pattern to include
reusability. And that
seems to be the core problem....that is if the Abstract class is
attempting to be
both an interface and a place to hold common methods and allow one to
say

Car *mySedan = factory->make( "Sedan" );

and not

Sedan *mySedan = factory->make( "Sedan" );

That seems to make a lot of differences. Basically it reduces to a
compile time problem...

Allow me to quote something from "Design Patterns" by Eric Gamma (,
.....),
Page 91, Abstract Factory.

"..... In fact with this approach, AbstractFactory only needs a single
"Make"
operation with a parameter indicating the kind of object to
create....."
.....
"This variation is easier to use in a dynamically typed language like
SmallTalk
than in a statically typed language like C++. You can use it in C++
only when
all objects have the same abtract base class or when the product
object can
be safely coerced to the correct type by the client that requested
them."
....
"But even when no coercion is needed, an inherent problem remains: All
products are returned to the client with the same abstract interface
as
given by the return type. The client will not be able to differentiate
or make
safe assumptions about the class of a product. If clients need to
perform
subclass specific operations, they won't be accessible through the
abstract
interface. Although the client could perform a downcast, that is not
always
feasible or safe, because the downcast can fail. This is the classis
trade-off for a highly flixiable and extensible interface."

end of quote....

Which is what I'm been having problem with all along.....if my client
say

Car *mySedan = factory->make( "Sedan" )

althought factory has returned an instance of Sedan, a downcast has
taken
place and mySedan is only pointing to a Car type....on the other hand
if I ask
my client to say

Sedan *mySedan = factory->make( "Sedan" )

This works, but where is the abstraction here....I might as well as
the client
to say

Sedan *mySedan = new Sedan()

The idea is to distance the client from specifics....hide the
implementation of
concrete classes....so was the motivation for Abstract pattern....

So perhaps I should be looking at another pattern for my problem...
I'll post my question a different way in the next thread....

Kevin Goodsell <us*********************@neverbox.com> wrote in message news:<3f4bbce4@shknews01>...

(Note: Excuse my replying to a reply rather than the actual post. It has
not arrived on my server.)

Perhaps I should include some code, maybe I'm failing to express my
problems...I'll present a Car.cc, Sedan.cc and Pickup.cc. Which is
a small change from Car, Ford, Toyota...but I wanted to show how
some cars might have different functionalities such as a Pickup truck
has openTailGate() where Sedan has openRearDoor()...

// -------------------- here is Car.h -----------------
#ifndef _CAR_H_
#define _CAR_H_
This is illegal. Identifiers beginning with an underscore followed by an
uppercase letter are reserved for the implementation, and you may not
use them.

Always assume that the compiler has #defined identifiers like this to
something like system("format c: /u /y") or something equally terrible.
You'll be lucky if it doesn't compile.

As a general rule, never begin an identifier with an underscore.
Sometimes it's legal, but it's much easier to just avoid it altogether
than to remember the exact rules. Also, you may not use an identifier
that contains two adjacent underscores.
#include <iostream>
#include <string>

using namespace std;
Please don't do this in a header file. You are polluting the global
namespace for everyone who attempts to use this header. If you want to
do it in your own .cpp files, that's one thing. But in a header that you
expect to be useful, you should not do this.

class Car
{
public:
// a factory method
Car * make(const string& carType = "" );

// a mandatory method for derived classes to implement
virtual void Drive() = 0;

// a common method without any data access...works
void greetDriver() { cout << "Hello driver." << endl ; }

// a common method with instance data access...don't work
void getColor() { cout << "Color=" << _color << endl; }

This is a compiler error. There is no _color defined in this class.

// car model specific methods
//virtual void openRearDoor();
//virtual void openTailGate();
};

#endif
// --------------------- here is Sedan.h ---------------------
#ifndef _SEDAN_H_
#define _SEDAN_H_
Still illegal.

#include <string>
#include "Car.h"

using namespace std;
Still a very bad idea.

class Sedan: public Car
{
public:
Sedan();
virtual ~Sedan() {};
Semi-colons don't go after functions.
void Drive();
void openRearDoor();

private:
string _color;
string _RearDoorState;
Also illegal.
};

#endif
// -------------------- here is Pickup.h -----------------
#ifndef _PICKUP_H_
#define _PICKUP_H_
Illegal.

#include <string>
#include "Car.h"

using namespace std;
Brain-damaged.

class Pickup: public Car
{
public:
Pickup();
virtual ~Pickup() {};
Drop the semi-colon.
void openTailGate();
void Drive();

private:
string _color;
string _TailGateState;
Illegal.
};

Yuck, why are you storing the state in a string? This is inefficient.

#endif
// ------------------- here is Car.cpp --------------
#include <iostream>
#include "Car.h"
#include "Sedan.h"
#include "Pickup.h"

using namespace std;

// ---------------- make() ------------
Car* Car::make(const string& carType /* = "" */ )
{
if ( carType == "Sedan" )
{
return( new Sedan() );
}
else if ( carType == "Pickup" )
{
return( new Pickup() );
}
else
{
cerr << "Error: You need to specify a car type" << endl;
return(NULL);
}
}
// ----------------------
// ------------------------ here is Sedan.cpp -------------
#include <iostream>
#include "Sedan.h"

using namespace std;

// ------------------------ Sedan() -----------------
Sedan::Sedan(): _color("White"), _RearDoorState("closed")
Illegal.
{
}
// --------------------- openRearDoor() -----------
void Sedan::openRearDoor()
{
_RearDoorState = "open";
Illegal.
cout << "Ok...Rear door is now open." << endl;
}
// --------------------- Drive() -----------
void Sedan::Drive()
{
cout << "Ok...I'm driving." << endl;
}
// ----------------
// ----------------- here is Pickup.cpp ----------------
#include <iostream>
#include "Pickup.h"

using namespace std;

// ------------------------ Pickup() -----------------
Pickup::Pickup(): _color("Black"), _TailGateState("closed")
Illegal.
{
}
// --------------------- openTailGate() -------------
void Pickup::openTailGate()
{
_TailGateState = "open";
Illegal.
cout << "Ok...Tail gate is now open." << endl;
}
// --------------------- Drive() -------------
void Pickup::Drive()
{
cout << "Ok...i'm driving." << endl;
}
// ---------------------
// ----------------------- here is the client foo.cpp -------------
#include <iostream>
#include "Car.h"
#include "Sedan.h"
using namespace std;

int main (int argc, char **argv)
{
cout << "Starting.... " << endl;

Car *factory ;
Car *mySedan = factory->make( "Sedan" );
Here you dereference an uninitialized pointer. That is very, very bad.
Car *myPickup = factory->make( "Pickup" );

mySedan->greetDriver() ;
//mySedan->openRearDoor();

myPickup->greetDriver() ;
//myPickup->openTailGate() ;

delete ( mySedan );
delete ( myPickup );
}
// ----------------------- here is the compiler error --------
medi@medi:~/cpp/car> make foo
g++ -c foo.cpp Car.cpp Sedan.cpp Pickup.cpp
In file included from foo.cpp:2:
Car.h: In member function `void Car::getColor()':
Car.h:21: `_color' undeclared (first use this function)
Car.h:21: (Each undeclared identifier is reported only once for each function
it appears in.)
In file included from Car.cpp:2:
Car.h: In member function `void Car::getColor()':
Car.h:21: `_color' undeclared (first use this function)
Car.h:21: (Each undeclared identifier is reported only once for each function
it appears in.)
In file included from Sedan.h:5,
from Sedan.cpp:2:
Car.h: In member function `void Car::getColor()':
Car.h:21: `_color' undeclared (first use this function)
Car.h:21: (Each undeclared identifier is reported only once for each function
it appears in.)
In file included from Pickup.h:5,
from Pickup.cpp:2:
Car.h: In member function `void Car::getColor()':
Car.h:21: `_color' undeclared (first use this function)
Car.h:21: (Each undeclared identifier is reported only once for each function
it appears in.)
make: *** [foo] Error 1
medi@medi:~/cpp/car>

I didn't look at the code all that closely, but I'd say the message to
which I'm replying identified the main problem that the compiler is
complaining about.

-Kevin

[Kevin Goodsell]

Medi Montaseri wrote:

Thank you very much Kevin for your comments....

Please don't top-post.

<snip explanation of factory pattern>

I understand the pattern you are trying to use. What I don't understand
is why you do something like this:

Car *factory;
Car *c = factory->Make("Sedan");

This is clearly illegal. You could make 'Make' a static function, then
do this:

Car *c = Car::Make("Sedan");

But I don't really think it makes sense to have 'Make' be a member of
'Car' at all. After all, cars don't make other cars. I'd probably have a
non-member function called CarFactory() or something like that. But
either way should work.

The thing you seem to be concerned about is the use of Car *, meaning
you only have access to the object via Car's interface, and any
additional members of the actual object are inaccessible. Well, that's
how these things work. Car should provide a common interface with pretty
much everything you might want to do to a car. If that's not what you
want, then this may not be the right approach.

But the question is, what approach would make you happy? You want to
deal with cars, and have different car sub-types have different
interfaces? OK, you can do that, but how are you going to write code to
use them? If the interface were common, you could write code that only
deals with Cars, so it handles all sub-types without you needing to
worry about it. With different interfaces, you have to deal with each
type individually, which is quite messy, and makes adding new Car types
much more difficult.

-Kevin
--
My email address is valid, but changes periodically.
To contact me please use the address from a recent posting.

[Medi Montaseri]

Kevin Goodsell <us*********************@neverbox.com> wrote in message news:<3f4ce718@shknews01>...

Medi Montaseri wrote:
I understand the pattern you are trying to use. What I don't understand
is why you do something like this:

Car *factory;
Car *c = factory->Make("Sedan");

This is clearly illegal. You could make 'Make' a static function, then
do this:

Car *c = Car::Make("Sedan");
Actually it is not illegal....you can reference member of an abstract class
via pointers and references. If Car is an abstract class then above is correct.
But I don't really think it makes sense to have 'Make' be a member of
'Car' at all. After all, cars don't make other cars. I'd probably have a
non-member function called CarFactory() or something like that. But
either way should work.
The reason for Car::Make() is to have a factory method where an instance
of the sub-type (particular car) is returned based on some parameters fed
to Make(). Some people wite it like

Car *c = factory->MakeSedan()
Car *d = factory-MakePickup()

In this approach, compiler does the switching and in my approach Make()
parses and does the switching, ie parameter driven. But the problem with
my approach is that my return type can not be known in advance and I"ll
have to return Car * which implies a down-cast. That is why C++ folks
use the following

Sedan *c = factory->MakeSedan()

Which allows you to explicitly declare the return type of MakeSedan() a
Sedan pointer.
But the question is, what approach would make you happy? You want to
deal with cars, and have different car sub-types have different
interfaces? OK, you can do that, but how are you going to write code to
I'd like to provide a common Interface to (like an API) instantiating cars and
using the instances as well.

Services provided by such an API should be reachable via the common
interface. However I would like to tap into powers of virtual functions so
that specific services are implemented by sub-types, instead of a handle-body
pattern where generic car forwards to sub-type, as this arrangement would
require updating Car everytime sub-type has a new service to offer.

Having said that I'm begining to see that asking the client to say

Case I) Sedan *mySedan = factory->MakeSedan()
vs
Case II) Car *mySedan = factory->Make ( "Sedan" );

has not provided much ROI. In both cases the client has to hint the interface
that he/she is interested in an instance of a "Sedan". So I might as well go with
Case I and not worry about coersing (down-casting) issues.

So to recap (or confirm my understanding) I'll use an abstract class Car to
provide a factory method as well as a place to mandate what has to be
implemented by sub-types. I'll also use the abstract base class as a
container of common methods used by all sub-types.

Does that sound about right...
use them? If the interface were common, you could write code that only
deals with Cars, so it handles all sub-types without you needing to
worry about it. With different interfaces, you have to deal with each
type individually, which is quite messy, and makes adding new Car types
much more difficult.

-Kevin

[Kevin Goodsell]

Medi Montaseri wrote:

Actually it is not illegal....you can reference member of an abstract class
via pointers and references. If Car is an abstract class then above is correct.
Please give a reference from the standard that permits dereferencing an
uninitialized pointer in this case. I don't see how your "justification"
is relevant.

The reason for Car::Make() is to have a factory method where an instance
of the sub-type (particular car) is returned based on some parameters fed
to Make(). Some people wite it like
I know the purpose of a factory function. I questioned the wisdom of
making it a member of the class. "Make a new car" is not an operation
one usually performs on a car, and cars don't generally contain
factories. The point is that it's not logical to make it a member. I'm
not saying you *can't*, I just think there may be a more logical place
to put your factory function.

Car *c = factory->MakeSedan()
Car *d = factory-MakePickup()

In this approach, compiler does the switching and in my approach Make()
parses and does the switching, ie parameter driven. But the problem with
my approach is that my return type can not be known in advance and I"ll
have to return Car * which implies a down-cast.
Why is the down-cast bad? It's the normal way things like this are done.
That is why C++ folks
use the following

Sedan *c = factory->MakeSedan()
*I* certainly don't do that (at least, not in general), and I doubt many
other "C++ folks" do, either.

Which allows you to explicitly declare the return type of MakeSedan() a
Sedan pointer.

But the question is, what approach would make you happy? You want to
deal with cars, and have different car sub-types have different
interfaces? OK, you can do that, but how are you going to write code to

I'd like to provide a common Interface to (like an API) instantiating cars and
using the instances as well.

Services provided by such an API should be reachable via the common
interface. However I would like to tap into powers of virtual functions so
that specific services are implemented by sub-types, instead of a handle-body
pattern where generic car forwards to sub-type, as this arrangement would
require updating Car everytime sub-type has a new service to offer.

So I don't see a problem.

Car *c = factory("Some Car Type");

/* Use c here. Functions called on it will automatically invoke the
correct function for the dynamic type of *c ("Some Car Type"). */

Having said that I'm begining to see that asking the client to say

Case I) Sedan *mySedan = factory->MakeSedan()
vs
Case II) Car *mySedan = factory->Make ( "Sedan" );

has not provided much ROI. In both cases the client has to hint the interface
that he/she is interested in an instance of a "Sedan". So I might as well go with
Case I and not worry about coersing (down-casting) issues.
What down-cast issues? There are no issues. II is the right answer. I is
far too restrictive. You're almost certain to run into problems with it.

So to recap (or confirm my understanding) I'll use an abstract class Car to
provide a factory method as well as a place to mandate what has to be
implemented by sub-types. I'll also use the abstract base class as a
container of common methods used by all sub-types.

Does that sound about right...

I don't know, the terminology you use confuses me. I would do this:

class Car
{
public:
void StartEngine() = 0;
/* ... */
};

class Sedan : public Car
{
public:
void StartEngine() {}
/* ... */
};

/* Add more car types */

/* This is a very simple factory function. */
Car *CarFactory(const std::string &type)
{
if (type == "Sedan")
{
return new Sedan;
}
else
/* ... */
}

int main()
{
std::cout << "What kind of car? ";
std::string car_type;
std::cin >> car_type;

Car *c = CarFactory(car_type);
c->StartEngine();

return 0;
}

-Kevin
--
My email address is valid, but changes periodically.
To contact me please use the address from a recent posting.

[Medi Montaseri]

Kevin Goodsell <us*********************@neverbox.com> wrote in message news:<3f4d99a4@shknews01>...

Medi Montaseri wrote:
What down-cast issues? There are no issues. II is the right answer. I is
far too restrictive. You're almost certain to run into problems with it.

Ok...lets use your example below to illustrate the down-cast
problem...

So to recap (or confirm my understanding) I'll use an abstract class Car to
provide a factory method as well as a place to mandate what has to be
implemented by sub-types. I'll also use the abstract base class as a
container of common methods used by all sub-types.

Does that sound about right...

I don't know, the terminology you use confuses me. I would do this:

class Car
{
public:
void StartEngine() = 0;
/* ... */
};

class Sedan : public Car
{
public:
void StartEngine() {}
/* ... */
};

/* Add more car types */

/* This is a very simple factory function. */
Car *CarFactory(const std::string &type)
{
if (type == "Sedan")
{
return new Sedan;
}
else
/* ... */
}

int main()
{
std::cout << "What kind of car? ";
std::string car_type;
std::cin >> car_type;

Car *c = CarFactory(car_type);
c->StartEngine();

return 0;
}

-Kevin

Can you now add some data members to this hierarchy....lets say there
are
two types of data members; common and specific (to a sub-type).
For simplicity, say all cars have a EngineState = { running or stoped
}.
Also lets say Sedan's have RearDoorState = { open or closed } while
Pickups have TailGateState = { open or closed } .

Let me see how you implement getEngineState(), getTailGateState() and
getRearDoorState()
such that I'd be able to say

Car *c = CarFactory( "Sedan" );
std::cout << "my car's engine state is " << c->getEngineState() <<
endl;
std::cout << "my car's rear door state is " << c->getRearDoorState()
<< endl;
Car *d = CarFactory ( "Pickup" );
std::cout << "my pickup's tail gate state is " <<
d->getTailGateState() << endl;

[Kevin Goodsell]

Medi Montaseri wrote:

Can you now add some data members to this hierarchy....lets say there
are
two types of data members; common and specific (to a sub-type).
For simplicity, say all cars have a EngineState = { running or stoped
}.
Also lets say Sedan's have RearDoorState = { open or closed } while
Pickups have TailGateState = { open or closed } .
You said Car was going to provide a common interface. If that's not the
case, and you want extra functions for specific types, then this
probably won't work.

Let me see how you implement getEngineState(), getTailGateState() and
getRearDoorState()
such that I'd be able to say

Car *c = CarFactory( "Sedan" );
std::cout << "my car's engine state is " << c->getEngineState() <<
endl;
std::cout << "my car's rear door state is " << c->getRearDoorState()
<< endl;
Car *d = CarFactory ( "Pickup" );
std::cout << "my pickup's tail gate state is " <<
d->getTailGateState() << endl;

If those functions are all members of Car then there's no problem. If
not, then I don't think the factory pattern is the right thing to use.
You can't possibly write a piece of generic code that can act on any Car
if it uses members that only exist in a sub-type of Car. That doesn't
make sense.

The question is, what do you really want to do? And is the factory
pattern going to help you do it?

-Kevin
--
My email address is valid, but changes periodically.
To contact me please use the address from a recent posting.

[Medi Montaseri]

Kevin Goodsell <us*********************@neverbox.com> wrote in message news:<3f4ef827@shknews01>...

The question is, what do you really want to do? And is the factory
pattern going to help you do it?

-Kevin

Ok....say you were to write an API for something....say a RAID controller.
A RAID controller because its hierarchical or compositional. ie a host
has one or many controllers, each controller has one or many logical disks
and each logical disk has one or many physical disks.

Logical Disks (or controllers for that matter) are a collection of similar but
not exactly the same entities. So one would think of a simple class hierarchy
consisting of a base and sub-types to implement RAID-0, RAID-1, etc.

Facade and Abstract Factory was identified as a good pattern such that
the API would have a consistent LogicalDisk Management Interface.
The same pattern can be applied throughout the sub-systems; Controllers,
LogicalDisks, PhsyicalDisks and the underlying operating system. eg
a base OS and derived Linux, NT, BSD, etc, etc

Again the constraint is that while controllers (or logicaldisks, etc) are
similar, they are different. That means each controller can have its own
attributes which may or may not even exists for the next controller.

Similarly LogicalDisks have different policies or even operations ....

So what pattern would fit this scenario...