Avoiding long member initializer lists

[TimmyT]

Hey guys,

to get used to object-oriented programming I started to setup a little text-based RPG and I need a little help with the structure and hope you can share your expertise.
The RPG includes heroes, which fight creatures (and may use items).

Therefore I decided to create an
Object class (Position, ClassName, Info)
-> Entity class (Willpower, Strength, Energy, n_Dice) derived from Object.
-> Hero class (Rank, Weapon, Gold Coins, Item Object, Name) derived from Entity.
-> Warrior, Dwarf, Sorcerer and Archer class, all derived from Hero.

So in the end a Warrior for example is defined by all these data members (Position, ClassName, Info, Willpower, Strength, Energy, n_Dice, Rank, Weapon, Gold Coins, Item Object, Name).

I initialized the members of the Object in its constructors initializer list as follows:

  Object::Object(const PositionMap& pos) : m_posXY(pos)
{
 m_ClassName="Object";
 m_Info="This is a physical object";
}

where PositionMap is another class containing the position on the map.
For the Entity follows:

Entity::Entity(const PositionMap& pos, int w, int s, int d)
	: Object::Object(pos), 
	m_Willpower(w), 
	m_Strength(s), 
	m_nDice(d)
{
 m_ClassName="Entity";
 m_Info="This is a physical, movable entity";
}

As you can already see here, the initializer list is getting quite long and will be even longer in the further inheritance process. Additionally, I have to add more and more parameters as arguments to the constructor.
For example, the constructor for the Hero class could be:

Hero::Hero(const PositionMap& pos, int w, int s, int d, const std::string& n, int e, int g, int r)
	: Entity(pos, w, s, d), 
	m_Name(n), 
	m_Energy(e), 
	m_GoldCoins(g),
	m_Rank(r)
{
 m_ClassName="Hero";
 m_Info="This is a hero";
}

This constructor has way too many parameters and as far as I know I have to transport these parameters as arguments of the constructor in order to be able to initialise them in the initialiser list, when I want to instantiate a Warrior object. If not, I couldnt access the private data members of the base classes. Is there a way to solve this more elegantly? I actually would prefer to have some initializations in the body of the constructor, so I wouldnt need to transport it as a constructor argument from object/entity down to the warrior class, but I read that you should always use the initializer list for initializations for performance reasons.


Second question concerns the data members m_ClassName and m_Info. I want to initialize them differently in each subclass as it is demonstrated above. Is there a way to do that outside the body of the constructor but instead in the initializer list without having it as an argument of the constructor?

Thank you for your help!
Cheers,
Timmy

[Lehti]

I'm afraid there isn't much you can do to avoid long initializer lists in this case. In general, the greater number of attributes, the greater number of parameters you may need to pass to that class' constructor whenever you instantiate an object.

One thing that comes to mind if you really want to pass less arguments to some (although not all) classes could be to assign a random value to some of its attributes. For example:

Hero::Hero(const PositionMap& pos, const std::string& n) : Entity(pos) {
    m_Energy = rand() % 50;
    //Add more random assignments
}

Considering that you're writing an RPG, using random number generation for each party member stats could be an idea.

Additionally, if you have, say, a class Enemy, you could provide some base stats in the initializer and multiply them by, for example, 1.0 + monster_level
This allows you to get "free enemies" with different stats depending on their level.

I don't think you need to derive multiple classes from Entity to represent party members: you can have the m_ClassName attribute as a private member of a Party_Member class and either
a) set its value directly in main() via an appropriate void set_ClassName(string name) function
b) declare a private static variable named party_size which starts at 1 and gets incremented whenever a new party member is added. Inside the constructor, you can use a switch case for every value it can take. The downside to this approach is that you'll always get a party member in a specific order you decided at the beginning.

If you do derive a class for every party member class, you can initialize them in the initializer list as string literals by using m_ClassName("Hero") and leaving the body empty by opening and closing brace, but I've never tried anything like this before, so I'm not really sure if it'll work.
To better exemplify, here's what it could look like

class Hero : public Entity {
    Hero() : m_ClassName("Hero"), m_Info("This is a hero") {}
}

[TimmyT]

Thank you for your detailed answer and suggestions!

I have an element of randomness, which are the dice. I intended to keep the initial stats the same for all heroes, however for different type of hero a different amount of dice (sorcerer has a rather high strength but only one die, archer rather low damage but more dice, only one counts etc.). This already gives me the possibility to initialise the values of the variables for all heroes in the hero class (except for dice), so I dont have to do that in the subclasses archer, sorcerer etc.
However, my enemy class is also derived from entity class. The enemies also contain the attributes willpower, strength, energy and dice, which is why I put it in the entity class. The enemies will have different initial stats of these variables than the heroes so I cannot do the initialization in the entity class.

The ClassName variable was intended to simply contain the name of the class, so rather a technical variable. I guess one could also just leave this one out, as I don't see any specific purpose for the RPG. What do you think?

I just came upon another problem, maybe you can also help me a little bit here:

At the beginning I want to make a query for the user to chose a hero, obviously, so I made a switch statement. This works fine, however, when I want to instantiate the chosen object, let's say a warrior, I do as follows:

std::string name;
std::cin >> name;
if (character == 'a') {
		Warrior* warrior = new Warrior(name);
}

, where 'character' denotes the char variable to chose between the heroes (a,b,c,d).
I have to use an if statement here, also for obvious reasons. My problem is that I cannot use the hero now out of scope, as it is instantiated within an if statement. What is the common, elegant way to get this done? Do I have to stay within this if statement for the rest of the RPG? That seems a bit inappropriate to me.

Cheers,
Timmy

[Lehti]

The ClassName variable was intended to simply contain the name of the class, so rather a technical variable. I guess one could also just leave this one out, as I don't see any specific purpose for the RPG. What do you think?

That's a fair point, actually.

At the beginning I want to make a query for the user to chose a hero, obviously, so I made a switch statement. This works fine, however, when I want to instantiate the chosen object, let's say a warrior, I do as follows:

1
2
3
4
5
std::string name;
std::cin >> name;
if (character == 'a') {
Warrior* warrior = new Warrior(name);
}


, where 'character' denotes the char variable to chose between the heroes (a,b,c,d).
I have to use an if statement here, also for obvious reasons. My problem is that I cannot use the hero now out of scope, as it is instantiated within an if statement. What is the common, elegant way to get this done? Do I have to stay within this if statement for the rest of the RPG? That seems a bit inappropriate to me.

Just declare the pointer to object before the if clause and instantiate a new object inside the if clause. Writing

std::string name;
Warrior *warrior;
std::cin >> name;
if (character == 'a') {
		warrior = new Warrior(name);
}

is perfectly functional C++ code, although it's not very elegant because, as you probably know, local variables are left uninitialized. So your warrior pointer will surely contain junk at the beginning. A better way to do this is by using:

std::string name;
Warrior *warrior = nullptr; //You're telling that warrior is a null pointer, i.e. a pointer that points to nothing
std::cin >> name;
if (character == 'a') {
		warrior = new Warrior(name);
}

I have to use an if statement here

You can also use a switch case statement, explained here at the bottom of the page: http://www.cplusplus.com/doc/tutorial/control/
I personally never use it because it can't work on ranges (only on single values) but for what you're doing, it works just fine.

[TheIdeasMan]

Hi,

If there are too many parameters required for a constructor, that is a hint that the design is not quite right. In this case you may be able to group together items into categories of related things. For example, you could have a Valuables base class with derived classes like Gold, Diamonds, Relics etc. Other more ephemeral things like Energy and Willpower might be grouped together. Then have a std::vector in which you push_back or emplace_back objects of the derived classes. This and other vectors then becomes a parameter to the constructor of the character being created, thereby having less parameters.

https://en.cppreference.com/w/cpp/container/vector/push_back
https://en.cppreference.com/w/cpp/container/vector/emplace_back

Investigate virtual polymorphism, the powerful thing about this is that a pointer to a derived class can appear where a pointer to base class is expected. So this allows a std::vector<Valubles> with say objects of type Gold, Diamonds being push_back or constructed in place with emplace_back into it.

http://www.cplusplus.com/doc/tutorial/polymorphism/

Really try not to use new , it's bad news because if an exception is thrown, neither the delete or the destructor is reached and memory is leaked. Prefer to use STL containers like std::vector which does it's own memory management on the heap efficiently and embracing the RAII concept. There are also smart pointers like std::unique_ptr which can be used with virtual polymorphism.

https://en.cppreference.com/w/cpp/memory/unique_ptr

The only time one would use new is for your own implementation of a list, or if you were making your own container like here, I call it an XMas Tree:

https://projecteuler.net/problem=18

Also look at design patterns, one of them is the Factory Pattern :

https://en.wikipedia.org/wiki/Factory_method_pattern

Also, consider naming the classes slightly differently. I would have a Base class Actor, then derived classes Hero and Enemy. I find this more meaningful, than an Entity class. Names are important in OOP design.

Good Luck !!