Is this correct / Suggestions?

closed account (1vD3vCM9)
Hello, I have tried to make my start of a Component Manager for all bunch of Components I want in my project.
I have almost never used Polymorphism so I want your thoughts about it.

The code is here:
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
#include <iostream>
#include <string>
#include <mutex>
#include <thread>
#include <algorithm>
#include <math.h>
#include <cstdio>
#include <vector>
#include <map>
#ifdef _MSC_VER
#define PRT_PTYFUNC std::cout << __FUNCSIG__ << "\n";
#else
#define PTR_PTYFUNC std::cout << __PRETTY_FUNCTION__ << "\n";
#endif

class Component {
public:
	virtual int Startup() = 0;
	virtual int Shutdown() = 0;
	virtual int Status() = 0;
	virtual ~Component() = 0 {}
};

class Is : public Component {
public:
	virtual int Startup() override final {
		PRT_PTYFUNC;
		return 0;
	}
	virtual int Shutdown() override final {
		PRT_PTYFUNC;
		return 0;
	}
	virtual int Status() override final {
		PRT_PTYFUNC;
		return 0;
	}
	Is() { PRT_PTYFUNC; }
	virtual ~Is() final { PRT_PTYFUNC; }
};

template<size_t count> class Component_Manager {
	std::vector<std::pair<int, Component*>> _Components;
public:
	Component_Manager() {
		_Components.resize(count);
		std::cout << "Size: " << _Components.size() << "\n";
	}
	Component* get_component(const int& index) noexcept {
		return _Components[index].second;
	}
	const Component& cget_component(const int& index) noexcept {
		return _Components[index].second;
	}
	void insert_component(Component* component) noexcept {
		_Components[_Components.size()-1].second = component;
	}
	void remove_component(const int& index) noexcept {
		_Components[index] = nullptr;
	}
	void insert_at(Component* component, const int& index) noexcept {
		_Components[index].second = component;
	}
	int size() noexcept {
		return _Components.size();
	}
	~Component_Manager() {
		for (size_t i = 0; i < _Components.size(); i++) delete _Components[i].second;
	}
};

class Another : public Component {
public:
	Another() { PRT_PTYFUNC; }
	virtual ~Another() final{ PRT_PTYFUNC; }
	virtual int Startup() override final { return 0; }
	virtual int Shutdown() override final { return 0; }
	virtual int Status() override final { return 0; }
};

auto main(int argc, char** argv) -> int {
	Component_Manager<5>* cp = new Component_Manager<5>();

	Another* another = new Another();
	cp->insert_component(std::move(new Is()));
	cp->insert_at(another, 3);
	delete cp;

	getchar();
	return 0;
}

/*
Prints:
Size: 5
__thiscall Another::Another(void)
__thiscall Is::Is(void)
__thiscall Another::~Another(void)
__thiscall Is::~Is(void)
*/


Please Tell me if there's something wrong or if I should do it in an other way (I did try to make the Component Manager a variadic templated class, but I could not have managed to make it work), Thanks.

(NOTE: This is not my final version of this code, that's also why I did not do the Big Five Rule)
Last edited on
oren drobitsky wrote:
I have almost never used Polymorphism so I want your thoughts about it.

I think runtime polymorphism is rarely useful. But if you do expect to make the decision, at runtime, such as based on some user input, whether an Another or an Is would be placed in the same Container_Manager, then this is indeed one of several possible implementation techniques.

cp->insert_component(std::move(new Is()));

that move does nothing: there's no rvalue overload of insert_component, and even if there was one, moving from a pointer (which is what new returns) is the same thing as copying a pointer.

More importantly, you shouldn't have a reason to use the keyword new in C++, especially in code that advertises itself to be C++14-only by using auto main -> int. Make your main function more like

1
2
3
4
5
6
7
int main()
{
  Component_Manager<5> cp;
  cp.insert_component(std::make_unique<Is>());
  auto another = std::make_unique<Another>();
  cp.insert_at(std::move(another), 3);
}
Last edited on
Whenever you are tempted to give something the name "Manager" ask yourself "what am I doing wrong here?" Why do you need an object manager? Is the C++ runtime not a good enough object manager for you?

In general, you want a factory or a pool or a container. You almost never want a manager.

My first recommendation is to write comments explaining why a class exists -- it's single responsibility. Managers rarely have single responsibilities and therefore violate the single responsibility principal.

Good class comments describe why a type exists (its single responsibility), how to use it properly (including limitations, lifecycle, thread safety, etc) and its invariants. I always start with comments, that way I know what I am doing and, when I seek help from others, they know my intentions.

In your case, your component manager is merely a container. Why not just use a standard container?
I wrote the above and what am I working on right now? A class called SyncManager. Yeah. I'm that guy.

What am I doing wrong here?

:-)
Just on the Single Responsibility thing, here is the acronym SOLID:

https://en.wikipedia.org/wiki/SOLID_(object-oriented_design)
virtual ~Component() = 0 {};
//ยง10.4/2 a function declaration cannot provide both a pure-specifier and a definition


http://stackoverflow.com/questions/8194571/pure-virtual-destructor-definition-inside-class-gives-compilation-error

I did try to make the Component Manager a variadic templated class

Why? Wouldn't you rather consider making Component a variadic class template?
Last edited on
Topic archived. No new replies allowed.