factory method

[jimryanabao]

hello, im trying to solve my problem using factory method but im not
sure if im doing it correctly this is my sample code

class Base {
public:
Base();
virtual ~Base;
virtual void draw();
};

class Item1 : public Base {
public:
Item1();
~Item1();
void draw();
};

class Item2 : public Base {
public:
Item2();
~Item2();
void draw();
};

// factory method ?!
Base * getItem(int code)
{
if (code == 1 || code == 2)
return new Item1();
else if (code == 3 || code == 4)
return new Item2();
else
return 0;
}

Base * item = getItem(code);
if (item)
item->draw();
delete item;

the sad part is, "code" should have atleast 50 possible values and i
have atleast 8 classes derived from a single base class. am i doing
this right?! any suggestions would be appreciated, thanks...

[AbdulMunaf]

Well if you have 8 different items to draw then you have to impilement
8 different classes. If there are some common things you are doing in
those 8 classes then you can write some utility class and use it.

Also you can improve the getItem() like this:

Base * getItem(int code)
{
Base* newInstance = 0;
seitch( code )
{
case 1:
case 2: //You may add some more casese for which u have
return Item1.
newInstance = new Item1();
break;

case 3:
case 4: //Same here as above comment.
newInstance = new Item2();
break;

default:
newInstance = 0;
break:
}

return newInstance;
}

[Rolf Magnus]

ji*********@gmail.com wrote:

hello, im trying to solve my problem using factory method but im not
sure if im doing it correctly this is my sample code

class Base {
public:
Base();
virtual ~Base;
virtual void draw();
};

class Item1 : public Base {
public:
Item1();
~Item1();
void draw();
};

class Item2 : public Base {
public:
Item2();
~Item2();
void draw();
};

// factory method ?!
Base * getItem(int code)
{
if (code == 1 || code == 2)
return new Item1();
else if (code == 3 || code == 4)
return new Item2();
else
return 0;
}

Base * item = getItem(code);
if (item)
item->draw();
delete item;

the sad part is, "code" should have atleast 50 possible values and i
have atleast 8 classes derived from a single base class. am i doing
this right?! any suggestions would be appreciated, thanks...

You could replace the if/else cascase with switch/case, wich makes it a bit
more readable IMHO:

switch (Code)
{
case 1:
case 2:
return new Item1();
case 3:
case 4:
return new Item2();
default:
return 0;
}

Generally, you should use an enum or constants instead of magic numbers.
Another possibility is to make the classes register with the factory. Then
you can write your program in such a way that the factory doesn't need to
know which derived classes exist.

[ma740988]

ji*********@gmail.com wrote:

hello, im trying to solve my problem using factory method but im not
sure if im doing it correctly this is my sample code

class Base {
public:
Base();
virtual ~Base;
virtual void draw();
};

class Item1 : public Base {
public:
Item1();
~Item1();
void draw();
};

class Item2 : public Base {
public:
Item2();
~Item2();
void draw();
};

// factory method ?!
Base * getItem(int code)
{
if (code == 1 || code == 2)
return new Item1();
else if (code == 3 || code == 4)
return new Item2();
else
return 0;
}

Base * item = getItem(code);
if (item)
item->draw();
delete item;

the sad part is, "code" should have atleast 50 possible values and i
have atleast 8 classes derived from a single base class. am i doing
this right?! any suggestions would be appreciated, thanks...

I'm wrestiling with something similar to what you're doing so I'm
curious to see how the more seasoned veterans here respond. Implement
a 'Collection' - if you will class. So now:

#include <iostream>
#include <functional>
#include <algorithm>
#include <cassert>
# include <vector>

using namespace std;
class Base;
class Item1;
class Item2;

class ItemInteface
{
public:
virtual void draw( Base& ) { assert(false && "Not implemented."); }

virtual void draw( Item1& ) { assert(false && "Not implemented"); }

virtual void draw( Item2& ) { assert(false && "Not implemented"); }

virtual ~ItemInteface();
} ;

class Base {
public:
Base();
virtual ~Base();
virtual void draw(ItemInteface& i ) { i.draw (*this); }

};
class Item1 : public Base {
public:
Item1();
~Item1();
virtual void draw(ItemInteface& i ) { i.draw (*this); }
};

class Item2 : public Base {
public:
Item2();
~Item2();
virtual void draw(ItemInteface& i ) { i.draw (*this); }
};

#if 0
// factory method ?!
Base * getItem(int code)
{
if (code == 1 || code == 2)
return new Item1();
else if (code == 3 || code == 4)
return new Item2();
else
return 0;
}
#endif

template<class T>
class Collection : public Base
{
vector<T*> t;
public:
Collection()
{
// determine some way to store base objects .. in t. this is 'a
hack' but
// hopefully illustrates the point
for (size_t i = 0; i < 2; ++i)
t.push_back( new T );
}
virtual void invoke( ItemInteface& toDraw ) // just invoke ..
{
cout << "Collection invoked.\n";
for (size_t i = 0; i < t.size(); ++i)
t[i]->draw( toDraw );
}
};
So you have a Collection of 'items' if you will.
An aside:
The difference between what you've alluded to and what I'm trying to do
surrounds the fact that each of my _items_ takes different arguments.
This complicates my 'draw' function ( in mine it's called 'process' )
so I'm stuck.