Over many years, I always find myself reconsidering this design, so I wanted to get some feedback on my solution to it.
Problem:
I need a limited amount of objects = instances from a class, and I don't want to expose the option to create more. I also want easy access to them from everywhere, and operators like equal / not equal.
They need to be class instances, as they need to have methods, so a enum doesn't work.
(Simplified) Example:
Consider for example any chess-like game.
- There are three tile colors for the board needed, Lighter and Darker, and a different size tile where taken pieces are put. Also,
- the pieces have a color, typically White or Black; and I use NoColor for empty fields.
Both examples, each instance should know how to draw and serialize itself.
Obvious Option:
If I make a simple enum, the values cannot carry methods, so I end up with Draw and Serialize methods in some other class, which are basically an ugly switch/case.
My Solution:
I like to
- protect the constructor, and,
- create (public) static instances inside the class (C++17 now allows that even directly with
inline).
'Each instance gets created with a const 'type' in it (basically a enum), set through the constructor.
Wherever I need the information, I use the const pointer to that static instance. This allows to easily _compare and assign them, and also allows calling their member functions as needed.
It even allows other games to derive from the class and add new static instances, for example, a three-player chess could add a Red instance inside a derived class. Because I hand around pointers to the base class everywhere, it is polymorphic, and the code doesn't have to handle the new color special in any way...
class Color
{
protected:
Color(char c) : color{c} {}
public:
void Serialize(Archive& ar) const { ar << color; }
private:
const char color;
public: // the only instances ever to exists; handed around by pointer
inline const static Color NoColor{'X'}; // C++17 syntax, otherwise
inline const static Color White{'W'}; // separate line in .cpp needed
inline const static Color Black{'B'};
};
class Chess3Color : public Color
{
protected:
Chess3Color(char c) : Color{c} {}
public:
inline const static Chess3Color Red{'R'};
};
Example Usage:
...
// create a White King
Piece* p = new WhiteKing{&Type::King, &Color::White, IDB_W_K_BMP};
...
if (p->IsColor(&Color::White)) { ... }
...
p->Serialize(ar);
...
class Piece
{
...
// Piece member function for serialization
void Serialize(Archive& ar)
{
type->Serialize(ar);
color->Serialize(ar);
}
...
// check if this piece is of a certain color
// note that pointer compare is good enough!
bool IsColor(const Color* c) { return color == c; }
...
}
Question:
I go back and forth in my mind between this being a code smell or a pretty good and clean solution. So is it a code smell?
How else would this be better designed?
I have read a lot about Singletons, and I don't think they would do me any good (aside from the fact that some people think they are a code smell too).
