Reputation: 2958
I have a map of that maps string ids to a specific implementation of base_object. Any base_object has a method get_id that returns the id of the object. and I fill up the map using (pseudo)
void addToMap(base_object* obj){
make_pair(obj->get_id(),obj);
}
This map acts like a factory, and knows to return the correct object using the id. this is the method declaration for retrieving specific object:
base_object* get(string id);
All the objects do not have fields only a polymorphic method that behaves differently for each implementation.
I am not sure what is the best way to implement this.
Should the map be a map of <string,unique_ptr<base_object>>
In this case when I am returning a base_object using get, is it ok to return a raw pointer to base_object? (I know that the map will keep living so that the object will not get destroyed?)
or maybe in this case I should use a shared_ptr?
Also, since the object doesn't really have any fields, maybe it is better to return a copy of the object?
Any way I look at this it looks to me like a bad design, and I just can't decide what is the best approach to solve this. I am very new to cpp so I am not very familiar with all the differences and best usages of pointers...
Upvotes: 0
Views: 2567
Reputation: 23001
You can use std::unique_ptr<base_object>
and return a const reference to the unique_ptr
.
Possible implementation:
struct Data
{
std::map<std::string,std::unique_ptr<base_object>> data;
void add(base_object* obj){
data[obj->get_id()] = std::unique_ptr<base_object>(obj);
}
const std::unique_ptr<base_object>& get(const std::string& id) {
return data.at(id);
}
};
Use case example:
Data data;
data.add(new test1_object{});
data["test1"]->create(); // call a virtual function of base_object
Note, that this is not really a factory. If the abstract function of base_object
should be responsible for creating your actual product, you can perhaps do this:
struct Factory
{
std::map<std::string,std::unique_ptr<base_object>> workers;
void add(base_object* obj){
data[obj->get_id()] = std::unique_ptr<base_object>(obj);
}
Product create(const std::string& id) {
return data.at(id)->foo(); // call the virtual function here
}
};
Factory factory;
factory.add(new test1_object{});
Product x = factory.create("test1");
Upvotes: 1
Reputation: 45664
How you best design your thing depends on the use-cases and design constraints:
std::atexit
for cleanup at the end), and let the map only hold an un-owning raw pointer.typeid
for the key (at least the name field) instead of a custom string.std::function
for maximum flexibility?std::unordered_map
has amortised O(1) access, a std::map
only O(ln(n)).Upvotes: 0
Reputation: 363627
Use unique_ptr<base_object> const &
. That signals to the caller that what it gets is a handle on a unique object having the id that it requested. Using a shared_ptr
signals that it may be responsible for keeping the object alive.
Also, there's no need for a map
: you can use a set
or unordered_set
that orders/hashes based on the id. That way, you won't have to store the id twice.
(The thing you're implementing is more of an object pool than a factory.)
Upvotes: 1
Reputation: 25623
The standard of the factory pattern is not filled up with objects on start. There are no objects at all. The factory only knows how to create a new object. A possible implementation can do this with a map and registered static! methods of the class ( not a object ).
And a factory should always return a new instance and not a reference or a pointer to an already existent object. The application typically have no idea how to destroy this special kind of copies instead of own instances.
Upvotes: 0