Return to Answer

Try not to get in the habit of using using namespace std. Read this for more information.

For clarity, have your #includes organized. Read this blog post or this answer 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:

Try not to get in the habit of using using namespace std. Read this for more information.

For clarity, have your #includes organized. Read this blog post or this answer 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:

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. If you end up implementing your map idea, that would be created in the constructor as well. 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. As for classMob, you could create a std::vector of those objects (you cannot predict how many monsters will be created until the player dies. New monsters would be added to the std::vector. If the player kills a monster, remove that monster. If not, keep it. This would also allow you to determine how many monsters the player has killed at the end of the game.

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.

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(std::time(NULL));

      Game game;
      game.play();
  }

That may be too little for main(), but the idea is that Game will handle everything. Every function in Game, except for play(), should be private. Game should contain a player as a data member and instantiated in Game's constructor. If you end up implementing your map idea (or anything similar), that would be instantiated in the constructor as well. You may still keep those extra driver functions, but they should be called in play() instead.

As for classMob, you could create an std::vector of objects (you cannot predict how many monsters will "spawn" before the player dies). New monsters would be added and, when killed by the player, removed. If you maintain a counter, you could even track the number of monsters killed before defeat.

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.

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. If you end up implementing your map idea, that would be created in the constructor as well. 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. As for classMob, you could create a std::vector of those objects (you cannot predict how many monsters will be created until the player dies. New monsters would be added to the std::vector. If the player kills a monster, remove that monster. If not, keep it. This would also allow you to determine how many monsters the player has killed at the end of the game.