Try not to get in the habit of using
using namespace std
. Read this this for more information.For clarity, have your
#include
s organized. Read this blog post or this answer 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
#include
s 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
#include
s 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()
andlevelUp()
, they don't need to return anything. Just make those functionsvoid
.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 amobClass
instance as an individual monster, just as aplayer
is just one player. As such, you just need to construct eachmobClass
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 aGame
and the class will handle the rest. Here's (roughly) whatmain()
could look like:int main() { std::srand(std::time(NULL)); Game game; game.play(); }
That may be too little for
main()
(and I'm probably leaving out some things), but the idea is thatGame
will handle everything. Every function inGame
, except forplay()
, should beprivate
.Game
wouldshould contain aplayer
, which would be created as a data member and instantiated inGame
's constructor. If you end up implementing your map idea (or anything similar), that would be createdinstantiated in the constructor as well. You may still keep those extra functions (the ones you already have in the driver) functions, but they wouldshould be part ofcalled inGame
instead.Playplay()
would then use those functions among the rest of the game's processesinstead. AsAs for
classMob
, you could create aanstd::vector
of those objects (you cannot predict how many monsters will be created until"spawn" before the player dies). New monsters would be added to thestd::vector
. Ifand, when killed by the player kills a monster, remove that monsterremoved. If notyou maintain a counter, keep it. This would also allow you to determine how many monsters the player has killed atcould even track the endnumber of the gamemonsters killed before defeat.
If you're just mutating data members in
calcEXP()
andlevelUp()
, they don't need to return anything. Just make those functionsvoid
.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 amobClass
instance as an individual monster, just as aplayer
is just one player. As such, you just need to construct eachmobClass
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 aGame
and the class will handle the rest. Here's (roughly) whatmain()
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 thatGame
will handle everything. Every function inGame
, except forplay()
, should beprivate
.Game
would contain aplayer
, which would be created inGame
'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 ofGame
instead.Play()
would then use those functions among the rest of the game's processes. As forclassMob
, you could create astd::vector
of those objects (you cannot predict how many monsters will be created until the player dies. New monsters would be added to thestd::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()
andlevelUp()
, they don't need to return anything. Just make those functionsvoid
.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 amobClass
instance as an individual monster, just as aplayer
is just one player. As such, you just need to construct eachmobClass
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 aGame
and the class will handle the rest. Here's (roughly) whatmain()
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 thatGame
will handle everything. Every function inGame
, except forplay()
, should beprivate
.Game
should contain aplayer
as a data member and instantiated inGame
'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 inplay()
instead.As for
classMob
, you could create anstd::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()
andlevelUp()
, they don't need to return anything. Just make those functionsvoid
.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 amobClass
instance as an individual monster, just as aplayer
is just one player. As such, you just need to construct eachmobClass
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 aGame
and the class will handle the rest. Here's (roughly) whatmain()
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 thatGame
will handle everything. Every function inGame
, except forplay()
, should beprivate
.Game
would contain aplayer
, which would be created inGame
'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 ofGame
instead.Play()
would then use those functions among the rest of the game's processes. As forclassMob
, you could create astd::vector
of those objects (you cannot predict how many monsters will be created until the player dies. New monsters would be added to thestd::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()
andlevelUp()
, they don't need to return anything. Just make those functionsvoid
.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 amobClass
instance as an individual monster, just as aplayer
is just one player. As such, you just need to construct eachmobClass
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 aGame
and the class will handle the rest. Here's (roughly) whatmain()
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 thatGame
will handle everything. Every function inGame
, except forplay()
, should beprivate
.Game
would contain aplayer
, which would be created inGame
's constructor. You may still keep those extra functions (the ones you already have in the driver), but they would be part ofGame
instead.Play()
would then use those functions among the rest of the game's processes.
If you're just mutating data members in
calcEXP()
andlevelUp()
, they don't need to return anything. Just make those functionsvoid
.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 amobClass
instance as an individual monster, just as aplayer
is just one player. As such, you just need to construct eachmobClass
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 aGame
and the class will handle the rest. Here's (roughly) whatmain()
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 thatGame
will handle everything. Every function inGame
, except forplay()
, should beprivate
.Game
would contain aplayer
, which would be created inGame
'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 ofGame
instead.Play()
would then use those functions among the rest of the game's processes. As forclassMob
, you could create astd::vector
of those objects (you cannot predict how many monsters will be created until the player dies. New monsters would be added to thestd::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.