Character classes and character creation for an RPG (early prototype)

Question

So, I've only started to learn actual programming about a week ago (only did some basic matlab scripting before) and because I wanted a challenge I decided to start with C++.

To get more familiar with the concept of classes and OOP in general, I started this little project, which is basically some classes that define characters in an RPG and also provides an interactive way of creating those. Well so far, it's really just a prototype, cause it doesn't nearly include all properties I want to assign to a character and the classes have some behaviour which isn't getting used (yet!). But since I'm really new to the whole thing, I thought: better let someone correct any bad practice early on before I get used to it.

The purpose of the program is (read as: will be) to take input from the user and output something (maybe a .txt file; not yet implemented) with all the necessary information about the characters he created. ANYHOW, I plan on also writing a little clone of the pen and paper game as a text-based video game later on and will then probably want to use the same classes/modules. That's why there are some methods in the classes, that the character creator doesn't use, but some type of game surely will/would.

So please correct anything that would stand in the way of the above mentioned purposes and any bad practice I'm using. Be as picky as you want, but please sort your advice in categories like "MUST CHANGE", "strongly recommended" and "recommended/subjective".

Also: I rarely (read as: virtually never) comment my code because I don't know whats worth commenting, and whats self-explanatory. So please recommend me some effective commenting behaviour for a) making myself remember what needs changing/iteration and b) making my code better understandable to others. I will edit my code appropriately if needed for easier reviewing.

So heres what I've got so far, this is what my file structure looks like:

• "RPGCreator_cpp"
  • "RPGGhoul_cpp"
    • "RPGGhoul_h"
    • "RPGAttr_cpp"
      • "RPGAttr_h"
    • "RPGDesc_cpp"
      • "RPGDesc_h"

and the file contents are the following: (link to online IDE)

"RPGCharCreator.cpp"

#include <vector>
#include "RPGGhoul.cpp"

int main() {
    std::vector <Ghoul> ghouls;
    std::string username = "unknown";
    int numghouls;

    std::cout << "Whats your (nick-)name?" << std::endl;
    std::getline (std::cin, username);
    for (;;) {
            std::cout << "How many ghouls do you want to create?" << std::endl;
            if (std::cin >> numghouls) {std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n'); break;}
            else {std::cin.clear(); std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');}
    }
    for (int i=1; i<=numghouls; i++) {
        std::cout << "\n***Creating Ghoul number " << i << " ***" << std::endl;
            ghouls.push_back(Ghoul(username));
    }
    for (int i=0; i<numghouls; i++) {
            std::cout << "\n***Created Ghoul number " << i+1 << " ***" << std::endl;
            ghouls[i].desc.dispName();
            ghouls[i].desc.dispDesc();
            ghouls[i].attr.dispAttr();
    }
    return 0;
}

"RPGGhoul.h"

#ifndef RPGGHOUL_H
#define RPGGHOUL_H

#include "RPGAttr.cpp"
#include "RPGDesc.cpp"
#include <string>
class Ghoul {
    public:
        RPGAttr attr{10};
        RPGDesc desc{"ghoul"};
        Ghoul (std::string);
    private:
        int d_b_min, d_b_max, i_b_min, i_b_max, c_b_min, c_b_max, e_b_min, e_b_max,
            p_b_min, p_b_max, a_b_min, a_b_max, l_b_min, l_b_max, s_b_min, s_b_max;
        const int AttrAv;
        std::string nameOfCreator;
        std::string type;
        std::string creature;
};

#endif

"RPGGhoul.cpp"

#ifndef RPGGHOUL_CPP
#define RPGGHOUL_CPP

#include "RPGGhoul.h"
#include "RPGAttr.cpp"
#include "RPGDesc.cpp"
#include <string>
#include <iostream>

Ghoul::Ghoul (std::string noc): d_b_min(5), d_b_max(13), i_b_min(6), i_b_max(14), c_b_min(5), c_b_max(12), e_b_min(7), e_b_max(16),
        p_b_min(7), p_b_max(13), a_b_min(7), a_b_max(15), l_b_min(4), l_b_max(12), s_b_min(6), s_b_max(14), AttrAv(10), nameOfCreator(noc) {
        std::string input;
        std::cout << "==========================================\n" << std::endl;
        for (;;) {
            std::cout << "Want to create a unique or generic character? (u/g)" << std::endl;
            std::getline(std::cin,input);
            type = input;
            if (type == "u" || type == "g"){break;}
        }
        if (type == "u"){
            std::cout << "------------------------------------------\n" << std::endl;
            desc.interactiveName(); std::cout << "------------------------------------------\n" << std::endl;
            desc.interactiveDesc(); std::cout << "------------------------------------------\n" << std::endl;
            attr.interactiveAttr_b_menu(d_b_min, d_b_max, i_b_min, i_b_max, c_b_min, c_b_max, e_b_min, e_b_max,
                                        p_b_min, p_b_max, a_b_min, a_b_max, l_b_min, l_b_max, s_b_min, s_b_max);
        }
}

#endif

"RPGAttr.h"

#ifndef RPGATTR_H
#define RPGATTR_H
#include <string>

class RPGAttr {
    public:
        RPGAttr (int);
        int D (); int I (); int C (); int E ();
        int P (); int A (); int L (); int S ();
        int TotalAttr_b ();
        int TotalAttr_ad ();
        int TotalAttr ();
        void incdecD (int);
        void incdecI (int);
        void incdecC (int);
        void incdecE (int);
        void incdecP (int);
        void incdecA (int);
        void incdecL (int);
        void incdecS (int);
        void setAttr_b(std::string,int);
        void interactiveAttr_b (std::string, int, int);
        void interactiveAttr_b_menu (int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int);
        void dispAttr();
        void dispMeaning();
    private:
        int default_av;
        int d_b,i_b,c_b,e_b,p_b,a_b,l_b,s_b,
            d_ad,i_ad,c_ad,e_ad,p_ad,a_ad,l_ad,s_ad;
        int limitAttr (int, int, int);
};

#endif

"RPGAttr.cpp"

#ifndef RPGATTR_CPP
#define RPGATTR_CPP

#include "RPGAttr.h"
#include <iomanip>
#include <iostream>
#include <limits>
RPGAttr::RPGAttr (int av):default_av(11), d_b(av), i_b(av), c_b(av), e_b(av), p_b(av), a_b(av), l_b(av), s_b(av),
                        d_ad(0), i_ad(0), c_ad(0), e_ad(0), p_ad(0), a_ad(0), l_ad(0), s_ad(0){}
int RPGAttr::D () {return limitAttr(d_b+d_ad,1,20);}
int RPGAttr::I () {return limitAttr(i_b+i_ad,1,20);}
int RPGAttr::C () {return limitAttr(c_b+c_ad,1,20);}
int RPGAttr::E () {return limitAttr(e_b+e_ad,1,20);}
int RPGAttr::P () {return limitAttr(p_b+p_ad,1,20);}
int RPGAttr::A () {return limitAttr(a_b+a_ad,1,20);}
int RPGAttr::L () {return limitAttr(l_b+l_ad,1,20);}
int RPGAttr::S () {return limitAttr(s_b+s_ad,1,20);}
int RPGAttr::TotalAttr_b () {return d_b+i_b+c_b+e_b+p_b+a_b+l_b+s_b;}
int RPGAttr::TotalAttr_ad () {return d_ad+i_ad+c_ad+e_ad+p_ad+a_ad+l_ad+s_ad;}
int RPGAttr::TotalAttr () {return D()+I()+C()+E()+P()+A()+L()+S();}
void RPGAttr::incdecD (int incdec) {d_ad=d_ad+incdec;}
void RPGAttr::incdecI (int incdec) {i_ad=i_ad+incdec;}
void RPGAttr::incdecC (int incdec) {c_ad=c_ad+incdec;}
void RPGAttr::incdecE (int incdec) {e_ad=e_ad+incdec;}
void RPGAttr::incdecP (int incdec) {p_ad=p_ad+incdec;}
void RPGAttr::incdecA (int incdec) {a_ad=a_ad+incdec;}
void RPGAttr::incdecL (int incdec) {l_ad=l_ad+incdec;}
void RPGAttr::incdecS (int incdec) {s_ad=s_ad+incdec;}
void RPGAttr::setAttr_b(std::string At,int val){
            if (At== "D") {d_b=limitAttr(val,1,20);} else if (At=="I") {i_b=limitAttr(val,1,20);}
            else if (At=="C") {c_b=limitAttr(val,1,20);} else if (At== "E") {e_b=limitAttr(val,1,20);}
            else if (At=="P") {p_b=limitAttr(val,1,20);} else if (At=="A") {a_b=limitAttr(val,1,20);}
            else if (At== "L") {l_b=val;} else if (At=="S") {s_b=limitAttr(val,1,20);}
}
void RPGAttr::interactiveAttr_b (std::string At, int Atmin, int Atmax) {
        if (Atmin>Atmax) {std::cerr << "Error: Invalid arguments for function Attributes::interactiveAttr_b_menu" << std::endl;}
            else {
                int ans;
                for (;;) {
                    std::cout << "What value should " << At << " be set to (" << Atmin << "-" << Atmax << ")?" << std::endl;
                    if (std::cin >> ans && (ans>=Atmin && ans <=Atmax)) {
                            setAttr_b(At, ans);
                            std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n'); break;}
                    else { std::cin.clear(); std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n'); std::cin.sync();}
                }
            }
}
void RPGAttr::interactiveAttr_b_menu (int dmin, int dmax, int imin, int imax, int cmin, int cmax, int emin, int emax,
                            int pmin, int pmax, int amin, int amax, int lmin, int lmax, int smin, int smax) {
        if (dmin>dmax || imin>imax || cmin>cmax || emin>emax || pmin>pmax || amin>amax || lmin>lmax || smin>smax) {
                std::cerr << "Error: Invalid arguments for function Attributes::interactiveAttr_b_menu" << std::endl;}
        else{
            int Atmin=1;
            int Atmax=20;
            std::string ans;
            int before = TotalAttr();
            for (;;){std::cout << "Want to change Attributes? (n for no; help for meaning of attributes; D,I,C,E,P,A,L,S for changing the respective Attribute)" << std::endl;
                    dispAttr();
                    std::cout << "Points to distribute (must equal 0 to proceed): " << before-TotalAttr() << std::endl;
                    std::getline (std::cin, ans);
                    if (ans=="n" && before-TotalAttr()==0) {break;}
                        else if (ans == "help") {dispMeaning();}
                        else if (ans == "D") {Atmin=dmin; Atmax=dmax;} else if (ans == "I") {Atmin=imin; Atmax=imax;}
                        else if (ans == "C") {Atmin=cmin; Atmax=cmax;} else if (ans == "E") {Atmin=emin; Atmax=emax;}
                        else if (ans == "P") {Atmin=pmin; Atmax=pmax;} else if (ans == "A") {Atmin=amin; Atmax=amax;}
                        else if (ans == "L") {Atmin=lmin; Atmax=lmax;} else if (ans == "S") {Atmin=smin; Atmax=smax;}
                    if ( ans == "D" || ans == "I" || ans == "C" || ans == "E" || ans == "P" || ans == "A" || ans == "L" || ans == "S" ) {
                    interactiveAttr_b(ans, Atmin, Atmax);
                    }
            }
        }
}
void RPGAttr::dispAttr(){
        int f=4;
        std::cout << std::left << std::setw(f) << "D" <<std::setw(f)<< "I" <<std::setw(f)<< "C" <<std::setw(f)<< "E" ;
        std::cout << std::setw(f) << "P" <<std::setw(f)<< "A" <<std::setw(f)<< "L" <<std::setw(f)<< "S" ;
        std::cout << std::setw(f+2) << "Tot." << std::endl;
        std::cout << std::left << std::setw(f) << D() <<std::setw(f)<< I() <<std::setw(f)<< C() <<std::setw(f)<< E() ;
        std::cout << std::setw(f) << P() <<std::setw(f)<< A() <<std::setw(f)<< L() <<std::setw(f)<< S();
        std::cout << std::setw(f+2) << TotalAttr() << std::endl;
}
void RPGAttr::dispMeaning(){
    int f=15;
    std::cout << std::left << std::setw(f) << "D: Dexterity" << std::setw(f) << "I: Intelligence" << std::endl;
    std::cout << std::left << std::setw(f) << "C: Charisma" << std::setw(f) << "E: Endurance" << std::endl;
    std::cout << std::left << std::setw(f) << "P: Perception" << std::setw(f) << "A: Agility" << std::endl;
    std::cout << std::left << std::setw(f) << "L: Luck" << std::setw(f) << "S: Strength" << std::endl;
}
int RPGAttr::limitAttr (int val, int lo, int hi) {
    if (val<=hi && val>=lo) {return val;} else if (val>hi) {return hi;} else if (val<lo) {return lo;} else {return 10;}
}

#endif

"RPGDesc.h"

#ifndef RPGDESC_H
#define RPGDESC_H
#include <string>
class RPGDesc {
    public:
        RPGDesc (std::string);
        void setName(std::string,std::string);
        void interactiveName();
        void dispName();
        void setDesc(std::string);
        void interactiveDesc ();
        void dispDesc();
    private:
        std::string firstname;
        std::string lastname;
        std::string shortdesc;
};

#endif

"RPGDesc.cpp"

#ifndef RPGDesc_CPP
#define RPGDesc_CPP

#include "RPGDesc.h"
#include <iostream>
#include <string>

RPGDesc::RPGDesc (std::string creature): firstname("Unnamed"), lastname(creature),
                                            shortdesc("a " + creature + " like any other"){}
void RPGDesc::dispName() {
    std::cout << "First name: " << firstname << std::endl;
    std::cout << "Last name: " << lastname << std::endl;
}
void RPGDesc::setName(std::string first, std::string last) {firstname = first; lastname = last;}
void RPGDesc::interactiveName (){
            std::string answer1;
            std::string answer2;
            std::string answer3;
            for (;;) {
                    std::cout << "Want to change name? (y/n)" << std::endl;
                    dispName();
                    std::getline (std::cin, answer1);
                    if (answer1=="n") {break;}
                    else if (answer1 == "y") {
                        std::cout << "Enter new first name" << std::endl;
                        std::getline (std::cin, answer2);
                        std::cout << "Enter new last name" << std::endl;
                        std::getline (std::cin, answer3);
                        setName(answer2,answer3);
                    }
            }
}
void RPGDesc::dispDesc() {std::cout << "Current description: " << shortdesc << std::endl;}
void RPGDesc::setDesc(std::string newdesc) {shortdesc = newdesc;}
void RPGDesc::interactiveDesc (){
            std::string answer1;
            std::string answer2;
            bool loop1=1;
            while (loop1==1){
                    std::cout << "Want to change description? (y/n)" << std::endl;
                    dispDesc();
                    std::getline (std::cin, answer1);
                    if (answer1=="n") {loop1=0;}
                    else if (answer1 == "y") {
                        std::cout << "Enter new description" << std::endl;
                        std::getline (std::cin, answer2);
                        setDesc(answer2);
                    }
            }
}

#endif

EDIT:

The online IDE is acting very differently from my CodeBlocks configuration using GCC...will see if I can find the reason for that...

EDIT2:

Using another online IDE now, which seems to work fine, but doesnt support multiple files, so I will just paste the contents as they would be pasted using the "#includes".

Answer 1

Must change

1. Do not include .cpp files. This is not only horrible practice, but will also lead to ODR-violations, which means that your program will have undefined behavior (assuming that it passes compilation and linking, which it currently does not). Make sure that your program compiles when all your .cpp files are passed as source file to the compiler, not just main.cpp.

Strongly recommended

1. Remove the include guards from your .cpp files. They are unnecessary (see point Must change/1).

2. Leave some space, vertically as well as horizontally. For example, let's look at

   if (At== "D") {d_b=limitAttr(val,1,20);} else if (At=="I") {i_b=limitAttr(val,1,20);}
   else if (At=="C") {c_b=limitAttr(val,1,20);} else if (At== "E") {e_b=limitAttr(val,1,20);}
   else if (At=="P") {p_b=limitAttr(val,1,20);} else if (At=="A") {a_b=limitAttr(val,1,20);}
   else if (At== "L") {l_b=val;} else if (At=="S") {s_b=limitAttr(val,1,20);}
   

   from RPGAttr.cpp. This is a horrible wall of text, and it is not even barely readable. What if you ever have to go back and change something in that if-block? Are you willing to spend most of your time and energy to parse that wall of text if you could spend it instead of getting on with your work? Why not opt for something more like

   if (At == "D") {
       d_b = limitAttr(val, 1, 20);
   } else if (At == "I") {
       i_b = limitAttr(val, 1, 20);
   } else if (At == "C") {
       c_b = limitAttr(val, 1, 20);
   } else if (At == "E") {
       e_b = limitAttr(val, 1, 20);
   } else if (At == "P") {
       p_b = limitAttr(val, 1, 20);
   } else if (At == "A") {
       a_b = limitAttr(val, 1, 20);
   } else if (At == "L") {
       l_b = val;
   } else if (At == "S") {
       s_b = limitAttr(val, 1, 20);
   }
   

   Much better, isn't it? Actually, seeing that At is only ever one character long, why not make it type char instead and use the convenience of a switch statement?

   switch (At) {
       case 'D':
           d_b = limitAttr(val, 1, 20);
           break;
       case 'I':
           i_b = limitAttr(val, 1, 20);
           break;
       case 'C':
           c_b = limitAttr(val, 1, 20);
           break;
       case 'E':
           e_b = limitAttr(val, 1, 20);
           break;
       case 'P':
           p_b = limitAttr(val, 1, 20);
           break;
       case 'A':
           a_b = limitAttr(val, 1, 20);
           break;
       case 'L':
           l_b = val;
           break;
       case 'S':
           s_b = limitAttr(val, 1, 20);
           break;
   }
   

   There are a lot of different opinions on when to leave space, but most developers agree that more whitespace is better than less.

3. Use telling variable names. What are

   int d_b,i_b,c_b,e_b,p_b,a_b,l_b,s_b,
       d_ad,i_ad,c_ad,e_ad,p_ad,a_ad,l_ad,s_ad;
   

   and how is anybody supposed to know? If you ever want somebody else to read and understand your code (such as in a code review), make their life not harder than it already is. Choosing one-character names for your variables is high up on the list of worst things you can do for that goal.

4. Use telling method and function names. Is void incrementOrDecrementD(int); really so much more of a nuisance to write than void incdecD(int);? Couldn't you have come up with a better name, e.g. void modifyD(int);? Finding good and descriptive names for things is hard. Still, that is no excuse to be sloppy and go with a name whose meaning you yourself might not remember half a year later when you have to make some changes to your code.

5. Avoid magic numbers.

   Ghoul::Ghoul (std::string noc): d_b_min(5), d_b_max(13), i_b_min(6), i_b_max(14), c_b_min(5), c_b_max(12), e_b_min(7), e_b_max(16),
           p_b_min(7), p_b_max(13), a_b_min(7), a_b_max(15), l_b_min(4), l_b_max(12), s_b_min(6), s_b_max(14), AttrAv(10), nameOfCreator(noc) {
   

   What does any of those numbers mean? Are they immutable? If yes, they should probably be constants with an own name and description. If not, they should probably be passed to the constructor as their own named parameters.

6. Do you know about SOLID? Especially the S (which stands for the Single Responsibility Principle) is important in your case. Concretely, you should split up the functionality of interacting with the user (getting/printing information) and game structure. For example, a Ghoul should not query information by means of std::cin from the user in its constructor, because that is not what the purpose of the Ghoul class is. Instead, you should encapsulate the idea of a Ghoul with all its attributes and actions that are important to you in it and leave the input to another class (whose sole purpose it is to fetch information from the user, ideally). This makes it easy to make changes to one part of the program only (for example, replacing the text interface with a graphical interface) while also unambiguously defining where what kind of function/method/attribute should live.

7. If you have a function taking a lot of parameters (usually more than four or five), chances are you are missing an abstraction (read the corresponding paragraph of the C++ core guidelines). For example,

   void interactiveAttr_b_menu (int,int,int,int,int,int,int,int,int,int,int,int,int,int,int,int);
   

   is terrible, because all 16 parameters have the same type and are unnamed. If you wanted to call this function correctly, you would likely have to look it up in its corresponding implementation file every single time, because remembering 16 different parameters and their correct order is nearly impossible (unless you waste the time and effort to memorize them, that is). Having the method take two RPGAttr as arguments would have been the much more readable and logical choice.

Recommended

1. Prefer while (true) over for (;;); it is more idiomatic and easier to read. However, having an explicit condition in your loop head is preferred to breaking out of the body in most cases.

2. Use unsigned and other integer data types where appropriate. As an example, numghouls is an int in your main-method. This allows the user to input a negative number as well, which your program handles gracefully by doing nothing and exiting. Still, it makes no sense to have -5 Ghouls, so numghouls should at the very least be unsigned.

3. Prefer pre-increment (and -decrement) over post-increment (-decrement) (i.e. ++x over x++). The reason for this is that, while it does not matter for integral types, it can incur a performance cost for types which overload both operators (this is because post-increment might need to make a copy, increment and then return said copy). So while it does not matter here, you should make it a habit to avoid unnecessary performance losses in different circumstances.

---

This review is not comprehensive. There is likely much more in your code that needs to be addressed, but as it currently is with code structure and readability, I am unwilling to spend my time analyzing the logic of your code (maybe someone else will be kind enough). I suggest you read my suggestions, then take a look at a good C++ book, then read the C++ core guidelines. Afterwards, you should improve your code and submit it for a second review.

Do not become discouraged, writing good code is hard.

Answer 2

I see a number of things that may help you improve your code.

Never #include cpp files

In C++ (as in C), header files (.h files) are intended to contain an interface and .cpp files are intended to contain an implementation. One significant difference is that a header file might be #included into multiple different files but a .cpp file should never be #included. Instead, each .cpp file is processed by the compiler to generate an object files (typically a .o file on Linux, or .obj on Windows). Then the linker combines those object files into the executable you're creating. Note that only .cpp files result in an object file, while header .h files do not (and should not). The advantage to doing things this way is that when your project grows, only the parts that need updating will actually be recompiled if you're using any kind of a modern build system. That's the reason you code did not compile in the online IDE.

Use all of the required #includes

Within RPGCharCreator.cpp, std::cout and std::string are used but the required #includes are not present. Fix that by adding

#include <string>
#include <iostream>
#include <limits>

Use more whitespace to enhance readability of the code

Instead of crowding things together like this:

if (std::cin >> numghouls) {std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n'); break;}

most people find it more easily readable if you use more space:

if (std::cin >> numghouls) {
    std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n'); 
    break;
}

Don't use std::endl if you don't really need it

The difference betweeen std::endl and '\n' is that '\n' just emits a newline character, while std::endl actually flushes the stream. This can be time-consuming in a program with a lot of I/O and is rarely actually needed. It's best to only use std::endl when you have some good reason to flush the stream and it's not very often needed for simple programs such as this one. Avoiding the habit of using std::endl when '\n' will do will pay dividends in the future as you write more complex programs with more I/O and where performance needs to be maximized.

Make it clear when loops end

The code currently uses for (;;) in a number of places, but it doesn't really loop infinitely. It's better, generally, to have the loop exit condition explicitly stated. For example, the loop in main() is actually simply trying to get a number from the user. It could be rewritten more clearly like this:

int numghouls{0};   // using C++11 initailization
while (numghouls == 0) {
        std::cout << "How many ghouls do you want to create?\n";
        std::cin >> numghouls;
        std::cin.clear(); 
        std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');}

Use const where practical

In the RPGAttr class, many of the functions only return a value without altering the underlying object. Such functions should be declared const as in:

void RPGAttr::dispAttr() const;

Rethink your objects

Right now, each Ghoul contains a collection of attributes. That's a good idea, but the implementation of it can be improved. Right now the RPGAttr class looks like this:

class RPGAttr {
    public:
        RPGAttr (int);
        int D (); int I (); int C (); int E ();
        int P (); int A (); int L (); int S ();
        int TotalAttr_b ();
        int TotalAttr_ad ();
        int TotalAttr ();
        void incdecD (int);
        void incdecI (int);
        void incdecC (int);
        void incdecE (int);
        // etc..

There are a couple of problems with this, but the most important one is that if, as you say, you're planning to add more attributes, this code will grow and grow and become a maintenance headache. Better would instead have two classes: RPGAttr which would be a single attribute and perhaps RPGAttrCollection which would, as the name suggests, be a collection of such attributes. If you do things that way, all of that repetition in the code will go away and you'll have a nice, clean, regular interface. I'd suggest doing something like this:

using RPGAttrCollection = std::array<RPGAttr, 8>;

Answer 3

For only recently having learned how to program, this is pretty good. So good, in fact, that I find it hard to believe you haven't been programming for longer than one week :)

I'll go along with your categories.

Must Change

• Unless you have a really good (and fairly evil) reason, never include a cpp. Just don't do it. Related to this, cpp files should never need #ifdef guards, because they should never be included.

• It's important that, wherever applicable, you mark things const. This occurs in a few different places. For instance, RPGAttr::D() should actually be D() const, indicating a promise or contract that that method does not mutate any member variables. Any member variables themselves that shouldn't change should also be marked const, probably to include those ints in RPGGhoul.

• In your headers, name your parameters. It's syntactically optional but very important for others (and future-you) to understand your code.

• Do not initialize a bool to 1. The only literals used should be true and false.

• Rather than while (somebool == 1), simply write while (somebool)

Recommended

• It's considered poor style to {do this; and that}. Statements should never be on the same line as a brace. It's OK (and a personal decision) whether to have an opening brace on its own line or to follow its corresponding if/else, but no style guide supports having multiple statements in one line, nor a statement and a closing brace on the same line.

• In your RPGGhoul.cpp, the post-parameter constructor initializers (d_b_min(5), etc.) can and should occur on one line each, to promote legibility.

Try addressing these and then attempt another round of review :)