patterncppMinor
Architecture for manager that gets shared among different components
Viewed 0 times
amongarchitectureshareddifferentcomponentsthatformanagergets
Problem
It started with a simple log manager. Then I wanted to implement more features, for example printing the name of the component that send a message. Later on, I can think of giving each component that prints log messages it's own text color in the terminal and options to mute or solo single modules.
Therefore, the manager needs the identity of the component that called its methods. So I decided to wrap the manager with an instance for every component. It tunnels function calls to the manager together with the name of the component.
```
#include
#include
#include
#include
using namespace std;
////////////////////////////////////////////////////////////////
// Manager declaration
////////////////////////////////////////////////////////////////
class manager {
public:
class instance;
instance &get_instance(string name);
void print(string sender, string message);
private:
unordered_map> m_instances;
};
////////////////////////////////////////////////////////////////
// Instance declaration
////////////////////////////////////////////////////////////////
class manager::instance {
public:
instance(string name, manager &manager);
void print(string message);
private:
string m_name;
manager &m_manager;
};
////////////////////////////////////////////////////////////////
// Manager implementation
////////////////////////////////////////////////////////////////
manager::instance &manager::get_instance(string name) {
if (m_instances.find(name) == m_instances.end())
m_instances[name] = unique_ptr(new instance(name, *this));
return *m_instances[name].get();
}
void manager::print(string sender, string message) {
cout << sender << ": " << message << "." << endl;
}
////////////////////////////////////////////////////////////////
// Instance implementation
////////////////////////////////////////////////////////////////
manager::instance::instance(string name, manager &manager) : m_name(name),
m_manager(manager)
Therefore, the manager needs the identity of the component that called its methods. So I decided to wrap the manager with an instance for every component. It tunnels function calls to the manager together with the name of the component.
```
#include
#include
#include
#include
using namespace std;
////////////////////////////////////////////////////////////////
// Manager declaration
////////////////////////////////////////////////////////////////
class manager {
public:
class instance;
instance &get_instance(string name);
void print(string sender, string message);
private:
unordered_map> m_instances;
};
////////////////////////////////////////////////////////////////
// Instance declaration
////////////////////////////////////////////////////////////////
class manager::instance {
public:
instance(string name, manager &manager);
void print(string message);
private:
string m_name;
manager &m_manager;
};
////////////////////////////////////////////////////////////////
// Manager implementation
////////////////////////////////////////////////////////////////
manager::instance &manager::get_instance(string name) {
if (m_instances.find(name) == m_instances.end())
m_instances[name] = unique_ptr(new instance(name, *this));
return *m_instances[name].get();
}
void manager::print(string sender, string message) {
cout << sender << ": " << message << "." << endl;
}
////////////////////////////////////////////////////////////////
// Instance implementation
////////////////////////////////////////////////////////////////
manager::instance::instance(string name, manager &manager) : m_name(name),
m_manager(manager)
Solution
About the architecture and thinking exclusively on the given example application (logging), I believe
If you plan using the
Please note that both simplifications above may not apply to other applications.
Now, the general code review comments (some of which were already applied on the snippet above):
The code is very well organized, very easy to read and understand. The coding style is consistent. I have only 3 suggestions for improvement:
Always pass
Avoid
Also, use braces after
manager does not need to track all manager::instance instances. Maybe in other applications this might be needed, but not in this one. Thus, I would just turn get_instance into a convenience method (make_instance) to create new manager::instances:class manager {
public:
class instance;
std::unique_ptr make_instance(const std::string& name);
void print(const std::string& sender, const std::string& message);
};
std::unique_ptr manager::make_instance(const std::string& name) {
return std::unique_ptr(new instance(name, *this));
}If you plan using the
manager like a singleton (having only one instance and everyone can access it), maybe the manager and manager::instance classes could be merge into a single one, where manager members would become static and manager::instance would become an actual instance of manager.Please note that both simplifications above may not apply to other applications.
Now, the general code review comments (some of which were already applied on the snippet above):
The code is very well organized, very easy to read and understand. The coding style is consistent. I have only 3 suggestions for improvement:
Always pass
std::string to a function as const std::string& instead of just std::string to avoid an unnecessary copy.Avoid
using namespace std;. If, for instance, you really want to use some std symbols unqualified, import them explicitly (like using std::string;). Also avoid using declarations on headers where they may affect user code (in global or namespace scope), otherwise it may break other's code when they include your header.Also, use braces after
if, while, do and for. Those kind of bugs (adding another indented line thinking it inside a block) are hard to spot.Code Snippets
class manager {
public:
class instance;
std::unique_ptr<instance> make_instance(const std::string& name);
void print(const std::string& sender, const std::string& message);
};
std::unique_ptr<manager::instance> manager::make_instance(const std::string& name) {
return std::unique_ptr<instance>(new instance(name, *this));
}Context
StackExchange Code Review Q#65122, answer score: 5
Revisions (0)
No revisions yet.