Skip to main content
Software Engineering

Return to Question

Bumped by Community user
Tweeted twitter.com/StackSoftEng/status/1184484282418585600
Bumped by Community user
improve formats
Source Link
edit approved May 23, 2018 at 6:13
Li Chen
edit approved May 23, 2018 at 6:13
Li Chen
  • 123
  • 4

Problem:

Problem:
I I need a limited amount of objects = instancesobjects = 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 an enuma enum doesn't work.

(Simplified) Example:

(Simplified) Example:
Consider Consider for example any chess-like game. (1) There are three tile colors for the board needed, Lighter and Darker, and a different size tile where taken pieces are put. Also, (2) the pieces have a color, typically White or Black; and I use NoColor for empty fields.
Both

  1. There are three tile colors for the board needed, Lighter and Darker, and a different size tile where taken pieces are put. Also,
  2. 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:

Obvious Option:
If 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:

My Solution:
I I like to a) protect the constructor, and b) create (public) static instances inside the class (C++17 now allows that even directly with inline). Each

  1. protect the constructor, and,
  2. create (public) static instances inside the class (C++17 now allows that even directly with inline).

'Each instance gets created with a const 'type'const 'type' in it (basically an enuma enum), set through the constructor.
Wherever I need the information, I use the const pointerconst pointer to that static instancestatic instance. This allows to easily compare and assign_compare and assign them, and also allows tocalling their call their member functionsmember functions as needed.
It even allows other games to derive from the class and add new static instances, for example, a three player-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...

Question:

Question: I 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?

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 an enum doesn't work.

(Simplified) Example:
Consider for example any chess-like game. (1) There are three tile colors for the board needed, Lighter and Darker, and a different size tile where taken pieces are put. Also, (2) 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 a) protect the constructor, and b) 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 an 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 to call 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..

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?

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.

  1. There are three tile colors for the board needed, Lighter and Darker, and a different size tile where taken pieces are put. Also,
  2. 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

  1. protect the constructor, and,
  2. 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...

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?

Source Link
Aganju
Aganju
  • 1.5k
  • 13
  • 15

How to improve a pattern 'enum class with methods'

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 an enum doesn't work.

(Simplified) Example:
Consider for example any chess-like game. (1) There are three tile colors for the board needed, Lighter and Darker, and a different size tile where taken pieces are put. Also, (2) 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 a) protect the constructor, and b) 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 an 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 to call 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).