Improving a factory function

[danep]

Hi all! This is my first time posting but this site has already been an invaluable resource- thanks for that!

I have a factory function that goes something like this (obviously simplified somewhat):

int const CAR_TOYOTA = 0;
int const CAR_FORD = 1;

Car CarFactory(int carType) {
 switch (carType) {
 case CAR_TOYOTA:
  return new Toyota;
 case CAR_FORD:
  return new Ford;
 }
}

This seems like a bit of a kludgy solution to me. The most annoying thing about it is that any time I add a new Car subclass, I need to first define a new constant and then add it to the factory function. Plus that switch statement gets awfully huge with a large number of Car subclasses. It seems like the factory function should be fairly "ignorant" about what it's producing. I should/want to be able to to something more like this (obviously even more simplified than the first case):

Car CarFactory(std::string carType) {
 return new carType;
}
// the carType parameter matches the name of a Car subclass 

I don't know how to express this concept in words very well, so I've had a tough time searching for solutions :) I'd appreciate any advice.

[blackcoder41]

i don't really know the goal of your class but if you are just trying to get rid of the switch statement then i suggest you use enumerations http://cplusplus.com/doc/tutorial/other_data_types/ or arrays or vectors..

anyway you should return a reference of the Car object in your function instead and subclasses (Java right?) are called Derived Classes in C++

[danep]

Thanks for correcting my terminology. My goal is to get rid of the switch statement as a means to an end- that is, allowing me to add new derived car classes without having to modify any of the "core" code. I don't want to have to define a new constant and/or modify a gigantic switch block in order to simply be able to create a new class.

Maybe this simply isn't possible? It just seems like it would be such a more elegant, robust, and portable solution, but maybe I have a much too idealized vision of how C++ works :)

[jsmith]

Your example is one of the typical uses of the factory design pattern. I don't think you will be able to remove
it. Or, we would have to understand how you are using the function to be able to suggest potential alternatives.

[danep]

Okay maybe if I give you a little more context it will make more sense- what I'm actually developing is a specialized video game platform of sorts. Each make of car is actually a game level- so the factory function returns a new Level of the type specified.

What I want is for other developers to be able to create new types of levels (derived classes of Level) and instantiate them without having to modify this core factory function.

[maikel]

What about template specialization? You could slightly improve the performance, because the switch wouldn't be necessary in runtime. You would have an "switch" at compile time.

Any objections to that idea?

[KrishnaV]

You can separate out the toyota/ford etc into other dlls and have the factory read an xml config file. In the file you can list the name of the dll to load for each car-type. And in the dll have a standard method (example: getNewInstance()) which will create the object and return its pointer.
So if the caller passes "toyota" to the factory, it will look up from the config what dll to load and then call the function "getNewInstance" on that dll.
That way you can add any number of car types and just have to update the config file - no change to core code whatsoever.

[helios]

Wow, now that's what I call a violation of Occam's Razor.

[jsmith]

At that point I'm not sure why the developer couldn't just instantiate the object directly without going
through a factory function.

[blackcoder41]

+1 helios
+1 jsmith

At that point I'm not sure why the developer couldn't just instantiate the object directly without going
through a factory function.

i wonder too

[KrishnaV]

it decouples the client from the underlying cartype objects.
it removes hardcoding of the cartypes from the factory implementation.
it removes the need to compile and link the factory with each and every cartype implementation.

infact if theres a GUI/Client that displays the cartypes supported and then after the user selects one of them creates an object and further interacts with the cartype via its interface, then the GUI can just extract the list of cartypes from the config file and when a user selects one of them it just passes it to the factory.

[danep]

Thanks KrishnaV! That's exactly the kind of solution I was looking for. It does sound complicated, but it also sounds like it exemplifies a better design pattern than the original factory function because it separates the functional/"business" code (which can be contained in the core executable) from the Level-specific content defined by end users. Anyway... now I just have to figure out the practicals of actually implementing it!

(I'll mark this as solved but I'd love to hear other opinions and more discussion on this topic)

[helios]

I would seriously reconsider. Not only is that unnecessarily complicated, it also forces the library user to use the same compiler that was used to build the library (because of C++ name mangling etc.).
If you want to build an object whose type depends on a string, this is a far better solution:

class FooFactory{
	virtual Foo *make(const std::string &);
	virtual bool register_type(const std::string &,Foo *);
	typename std::map<std::string,Foo *> types_t;
	static types_t types;
};

Foo *FooFactory::make(const std::string &name){
	types_t::iterator i=types.find(name);
	return (i==types.end())?0:i->clone();
}

FooFactory can be derived from if necessary, and more types can easily be added.

[danep]

Thanks helios. I guess what interested me about KrishnaV's idea was being able to read in content and parameters at runtime rather than relying so much on hardcoding them. But that's another issue entirely :)

I like your solution. So I suppose (something?) calls register_type and passes an instance of a Derived Foo, which gets added to the types_t map? If so, how exactly does register_type get called, and does this mean that types_t is carrying an instance of every possible derived class, even if that class would never otherwise be instantiated in the program?

[helios]

For this solution, that's unavoidable.
You can get rid of the extra instances by doing something like this instead:

Foo *FooFactory::make(const std::string &name){
	if (name=="Bar")
		return new Bar;
}

If someone wants to add new types, they'll have to override FooFactory::make().
Yet another solution would be to instead hold instances of 'FooMaker's that have a purely virtual function called make() that returns a Foo of the correct type. The problem is that would have to duplicate the class structure.

So, basically, either you make it polymorphic and hold possibly useless instances around, or you test using if. Personally, I think my original solution is the most elegant.

[KrishnaV]

thats right danep - the prototype like design pattern here would require you to register all implementations and you still need to figure out who the other something is that'll do the registration.

which is what moving the association between cartype and its implementation to an external file solves. your runtime memory footprint doesn't bloat unnecessarily, which in turn will improve your performance.
Also, one key point is that the cartype implementation is decoupled from the factory interface - it doesn't depend on any other interfaces. even after the product is deployed on a client's site, if someone independently developed and shipped a new cartype implementation dll, you could just drop that dll in and update your config file, or if there is a bug in one cartype implementation, you just ship an updated version of that dll alone. It just makes the system much more modular, decoupled and independent than monolithic.

in summary, it all depends on your requirements, there is no one size-fits all - the server apps we work on routinely need the kind of capabilities I mentioned here

[helios]

I just thought of a different solution. I honestly don't know why I didn't think of this until now, since I've used something similar before.

class FooFactory{
	virtual Foo *make(const std::string &);
	typedef Foo *(*FooMaker)();
	virtual bool register_type(const std::string &,FooMaker);
	typename std::map<std::string,FooMaker> types_t;
	static types_t types;
};

Foo *FooFactory::make(const std::string &name){
	types_t::iterator i=types.find(name);
	return (i==types.end())?0:(i->second)();
}

There, see? No useless instances, no cumbersome ifs, and no dynamic linking.

[Mythios]

This is probably nothing like you need. But another way would be having a register class that registers classes to a factory using a macro.

Use the factory function "Create" to create a car of a string type.

#include <iostream>
#include "CarFactory.h"

int main()
{
	std::cout << "Factory Demo\n";

	std::vector<Car*> carList;
	carList.push_back(CarFactory::Create("Car"));
	carList.push_back(CarFactory::Create("Ford"));
	carList.push_back(CarFactory::Create("Toyota"));

	for(unsigned int i = 0; i < carList.size(); i++)
	{
		carList[i]->CarPhrase();
	}

	return 0;
}

Simple Car class that is using the macro REG(Car) to register the "Car" class.

#ifndef _CAR_H
#define _CAR_H

#include <iostream>
#include <string>
#include "Register.h"

REG(Car)
class Car
{
	public:
		Car()
		{
		}
		~Car()
		{
		}

		void DisplayCarType()
		{
			std::cout << carType << "\n";
		}
		void SetCarType(std::string type)
		{
			carType = type;
		}
		virtual void CarPhrase()
		{
			std::cout << "Base Car - Has no sound.\n";
		}

	protected:
		std::string carType;
};

#endif//_CAR_H 

Again using the REG macro again but for our actual types of cars we'll use.

#ifndef _TOYOTA_H
#define _TOYOTA_H

#include "Car.h"
#include "Register.h"

REG(Toyota)
class Toyota : public Car
{
	public:
		Toyota() {}
		~Toyota(){}

		virtual void CarPhrase()
		{
			std::cout << "Oh what a feeling TOYOTA!\n";
		}
};

#endif//_TOYOTA_H 

#ifndef _FORD_H
#define _FORD_H

#include "Car.h"
#include "Register.h"

REG(Ford)
class Ford : public Car
{
	public:
		Ford() {}
		~Ford(){}

		virtual void CarPhrase()
		{
			std::cout << "FORD the way to drive!\n";
		}
};

#endif//_FORD_H 

Our CarFactory class which we use to actually add cars to the vector in main. Make sure we include the car/ford/toyota headers so we don't kill everything.

#ifndef _CARFACTORY_H
#define _CARFACTORY_H

#include "Car.h"
#include "Ford.h"
#include "Toyota.h"

#include <string>
#include <hash_map>
#include <vector>

class CarFactory
{
	typedef class Car* (*Func)();

	public:
		static Car* Create(std::string type);
		static void Add(Func createptr, std::string type);

	private:
		
		CarFactory();
		~CarFactory();

		static stdext::hash_map<std::string, Func>& getCarType()
		{
			static stdext::hash_map<std::string, Func> carTypes;
			return carTypes;
		}
};

#endif//_CARFACTORY_H 

#include "CarFactory.h"

CarFactory::CarFactory()
{
}
CarFactory::~CarFactory()
{
}

Car* CarFactory::Create(std::string type)
{
	std::cout << "Creating Car [" << type << "]...\n";
	return (*getCarType()[type])();
}

void CarFactory::Add(CarFactory::Func createptr, std::string type)
{
	getCarType()[type] = createptr;
}

And finally the register class. Which has the REG macro to register classes to our factory.

#ifndef _REGISTER_H
#define _REGISTER_H
#include "CarFactory.h"

#define REG(a) static const bool anon__##a = Register<class a>::_Register(#a);

template <class T> 
class Register
{
	public:
		static bool _Register( const char* a )
		{
			CarFactory::Add(Create, a);
			type = a;
			return true;
		}

		static class Car* Create()
		{
			Car* temp = new T;
			temp->SetCarType(type);
			return temp;
		}

	private:
		Register()  { }
		~Register() { }
		static std::string type;
};
template <class T> 
std::string Register<T>::type;

#endif//_REGISTER_H 

This probably looks a little confusing but I'm in a rush and quickly whooped it up!