* Try not to get in the habit of using `using namespace std`. Read [this](http://stackoverflow.com/questions/1452721/why-is-using-namespace-std-considered-bad-practice) for more information. * For clarity, have your `#include`s organized. Read [this blog post](http://blog.knatten.org/2010/07/01/the-order-of-include-directives-matter/) or [this answer](http://stackoverflow.com/questions/2762568/c-c-include-file-order-best-practices) for more information. * Add a newline between each section of code. For instance, separate all user input and loops. For variables, it's best to initialize them late as late as possible in case the function needs to terminate prematurely. Again, keep them with the corresponding code. * `mobclass.h` already includes `<string>`, so you don't need to include it again in the .cpp file. * You have a lot of accessors and mutators. Since these are short one-line implementations, you can define them in the header like this: void setEXP() {EXP = (getlevel() * 35;} int getEXP() const {return EXP;} As such, you will no longer need to implement these in the .cpp file. When they're in the header, they'll automatically be `inline`. It should also make it easier if you ever need to implement newer functions. In the header, you could also keep the accessors and mutators together for clarity. * I like what @Kaivo Anastetiks said about `classMob`'s constructor, but I would like to add on that a bit. You have a few options for this: 1. keep it in the .cpp file (with those changes) 2. put it in the header (with the `classMob::` part removed) 3. put it under the class declaration in the same file * It's better to use `getline()` instead of `cin` for getting an `std::string` value from the user: getline(std::cin, name); * `srand()` should ONLY be called ONCE in the program, preferably at the top of `main()`. If you keep it as is, `rand()` will be "not-so-random" because the seed will keep resetting to 0. * That really long output line in `battle()`'s do-while loop could be wrapped so that it doesn't extend out that far. * For `death()`: there's no need to have a function _just_ output a message. Either have it do something else relevant, or just remove it. * For the player's death (in general): I would prefer the function to fall back to `main()` instead of explicitly exiting. This is because: 1. it's clearer to let `main()` terminate the program whenever possible 2. it could be hard to tell where the player's death is determined You would then need to change `main()`'s loop to handle this. You could even have `battle()` return a `bool` to indicate the battle outcome (the player has won or has lost). Try this at the end: account = calcEXP(account, monster); if (account.getHealth() <= 0) return false; return true; You could even create a `bool` member function for determining if the player's health was depleted: bool healthDepleted() const {return playerHealth <= 0;} * If you're just mutating data members in `calcEXP()` and `levelUp()`, they don't need to return anything. Just make those functions `void`. * Your "saving" problem is due to your functions receiving the objects _by value_. It should be received _by reference_ instead. You were only passing in a copy and modifying it, only to have those changes discarded each time those functions ended. This change will allow you to modify the original objects: bool battle(player &account); void calcEXP(player &account, classMob &monster); * After looking at `mobClass`'s definition, it appears that you may not need those mutators. You should consider a `mobClass` instance as an _individual_ monster, just as a `player` is just _one_ player. As such, you just need to construct each `mobClass` once with the default stats. The accessors are still okay to keep. * Once again, I forgot about this: create a `Game` class. Since the human player doesn't need to know how the game's internal mechanisms work, you would no longer need those extra functions in the driver. Instead, `main()` will create a `Game` and the class will handle the rest. Here's (roughly) what `main()` could look like: int main() { std::srand(NULL); Game game; game.play(); } That may be _too_ little for `main()` (and I'm probably leaving out some things), but the idea is that `Game` will handle everything. Every function in `Game`, except for `play()`, should be `private`. `Game` would contain a `player`, which would be created in `Game`'s constructor. You may still keep those extra functions (the ones you already have in the driver), but they would be part of `Game` instead. `Play()` would then use those functions among the rest of the game's processes.