Abstract Factory or Factory Method pattern question....

Discussion in 'C++' started by Medi Montaseri, Aug 21, 2003.

  1. 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.... :)
     
    Medi Montaseri, Aug 21, 2003
    #1
    1. Advertising

  2. 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.
     
    Buster Copley, Aug 21, 2003
    #2
    1. Advertising

  3. Medi Montaseri

    Attila Feher Guest

    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
     
    Attila Feher, Aug 21, 2003
    #3
  4. 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
     
    Buster Copley, Aug 21, 2003
    #4
  5. Medi Montaseri

    Attila Feher Guest

    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
     
    Attila Feher, Aug 21, 2003
    #5
  6. "Attila Feher" <> wrote in message news:<bi1okm$jun$>...
    > 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...
     
    Medi Montaseri, Aug 21, 2003
    #6
  7. Medi Montaseri

    Attila Feher Guest

    Medi Montaseri wrote:
    > "Attila Feher" <> 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
     
    Attila Feher, Aug 22, 2003
    #7
  8. Buster Copley <> wrote in message news:<bi1sou$934$>...
    > 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.
     
    Medi Montaseri, Aug 22, 2003
    #8
  9. 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.
     
    Buster Copley, Aug 22, 2003
    #9
  10. Medi Montaseri

    Big Brian Guest

    > 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::eek:penRearDoor()
    > {
    > _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::eek:penTailGate()
    > {
    > _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.
     
    Big Brian, Aug 26, 2003
    #10
  11. (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::eek:penRearDoor()
    >>{
    >> _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::eek:penTailGate()
    >>{
    >> _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.
     
    Kevin Goodsell, Aug 26, 2003
    #11
  12. 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 <> 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::eek:penRearDoor()
    > >>{
    > >> _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::eek:penTailGate()
    > >>{
    > >> _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
     
    Medi Montaseri, Aug 27, 2003
    #12
  13. 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.
     
    Kevin Goodsell, Aug 27, 2003
    #13
  14. Kevin Goodsell <> 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
     
    Medi Montaseri, Aug 28, 2003
    #14
  15. 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.
     
    Kevin Goodsell, Aug 28, 2003
    #15
  16. Kevin Goodsell <> 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;
     
    Medi Montaseri, Aug 29, 2003
    #16
  17. 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.
     
    Kevin Goodsell, Aug 29, 2003
    #17
  18. Kevin Goodsell <> 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...
     
    Medi Montaseri, Sep 3, 2003
    #18
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. =?Utf-8?B?bWF4?=

    Design Pattern - Abstract Factory

    =?Utf-8?B?bWF4?=, Oct 18, 2005, in forum: ASP .Net
    Replies:
    2
    Views:
    671
    tdavisjr
    Oct 18, 2005
  2. Dave
    Replies:
    1
    Views:
    3,359
  3. sunny
    Replies:
    1
    Views:
    478
    Salt_Peter
    Dec 7, 2006
  4. C#
    Replies:
    4
    Views:
    434
  5. Replies:
    26
    Views:
    945
    Tom Anderson
    Dec 5, 2008
Loading...

Share This Page