Encapsulated text-based RPG using a randomized combat system

For my rags-to-riches submission, I've decided to improve this code:

Text-based RPG game using classes

However, I've decided to start off with something different. The original code is more interactive, but mine just runs automatically. I may consider making my code the same, but I wanted to try this first. I think my chosen approach is less error-prone as I don't have to worry about input validation and such.

I am proud to declare that my code appears more encapsulated than the original code, which was full of getters and setters. Mine does not have any at all, and instead uses inheritance, which was inspired by this answer to the same question. As it's my first real attempt at it, I'd especially like that aspect reviewed. I feel that my attempt isn't quite right, also because the derived classes don't have their own functions.

Other than inheritance, I'm concerned about maintainability. I will expand on this, such as by adding player/monster stats (attack and defense) and different monsters. I just didn't want to include it until I had this cleaned up first, especially if my use of inheritance could be problematic.

I've kept everything as integers to make things easier, and because RPG games already have numbers (such as health and experience) as integers. I'm also not sure if I should worry about making anything unsigned instead, except for health. I think it's okay for the player or a monster to end up with negative health after damage calculations, as it would still consider the creature to be dead.

I'm sort of playing around with the stats and combat numbers, which is also why the player ends up dying quickly sometimes (but they might still be reasonable). I do also plan on studying them further.

Please feel free to comment on everything. I want this to be cleaned up well at the start so that it's easier to keep it that way as it gets larger.

Test run on Ideone

LifeForm.hpp

```
#ifndef LIFEFORM_HPP
#define LIFEFORM_HPP

#include
#inclu

As it is, I can see an oversight in the level_up function: imagine that you need 5xp to level up. You gain 50xp, you level up, and your xp is 0 again. You just wasted 45xp! Here is the corrected function:

void level_up()
{
    level++;
    exp -= req_EXP;      // do not waste EXP
    req_EXP += level * 100;
    max_health += 50 * level;
    health = max_health;
}

That said, I don't really like the case of the name req_EXP. If you want your naming to be consistent, you should change it to req_exp. And the same applies to add_EXP.

You have useless elements in this loop:

while (!player.health_depleted() && !monsters.empty())
{
    std::cout  All monsters killed!!!";
    }

    if (player.health_depleted())
    {
        std::cout  Player dead...\n\n";
    }
}

The two conditions used to display messages will only appear once the whole battle is finished, and they are already checked in the while condition. Therefore, they should be moved out of the loop:

// Fight the monsters
while (!player.health_depleted() && !monsters.empty())
{
    std::cout  All monsters killed!!!";
}
else if (player.health_depleted())
{
    std::cout  Player dead...\n\n";
}

If you killed all the monsters, you can't be dead, so I added a little if to make it clear. Also, I would change the name of battle to something akin to play_turn since battle gives the feeling that we are playing the whole battle.

I also have the feeling that there is a problem in the function battle as is. This line:

current_monster().attack(player);

seems to always be executed, even if we just killed the last monster. In such a case, it appears to be calling pop_back on an empty std::vector, which is undefined behaviour. I don't think I am mistaken, so you should put a condition before executing that last instruction.

One late remark: design-wise, I don't really like that function signature:

void add_EXP(LifeForm const& life_form);

I would have changed it to:

void add_EXP(int exp);

and added a function exp_gained or exp_reward or something like that to LifeForm. That allows you to add new means of earning experience points without having to change the class LifeForm (rewards from quests for example). It makes it easier to add new elements to the game without having to modify those which already exist. Add it makes the name add_exp more consistent.

And yet another small error: if you want damages to remain from one turn to another, current_monster should return a Monster& and not a Monster. Otherwise, you are merely trying to kill a temporary monster; in other words, a monster that heals between turns. You probably just introduced this error with your last edit.

Monster& current_monster()
{
    return monsters.back();
}

Actually, you could do that and provide a const overload that returns by value:

Monster current_monster() const
{
    return monsters.back();
}

StackExchange Code Review Q#57013, answer score: 9