Recommended Posts

C++ using VS 2005
Far from finished, but I have run into a problem. Also critique my newb code
Problem: When attack (The only function other than quit that works) the player and enemy will have the same amount of attack points against each other, ie
I hit attack and hit him for 2 points, he attacks me for 2 also. I attack him again, I hit him for 4 points and he hits me for 4.. etc.
The code where this is stored is the attack() functions for player and enemy
code:

However, if you have members it is a good idea to initialise them. So, I would recommend omitting the destructors and filling out the constructor by choosing a good starting value for each member variable. Don't forget to use initialiser lists in the constructor (google the term if you haven't come across it yet).

Second, when in a member function, you don't need to qualify member variables with the class name. Put more simply, player::attack could be implemented like this:

Next, the health of the player/enemy is surely a good candidate for a member?

While we are discussing what is a suitable member, should we be storing "attackPoints"? After all, its value is only used in player::attack, and it is returned from there. With a little bit of work inside main(), it could be made a local:

The "menu choice" is a poor member variable, because it is used only inside main(), and is only tangentially related to the player. I would simply make it a local there. This is a recurring theme: temporary values (such as "choice", and "damage for a single attack") should be locals. Members are reserved for long term state (such as "health").

Next, lets look at it from an OO view. We have enemies, and players. However, apart from having different variable names, the code looks remarkably similar. We can take advantage of this to reduce the repetition. We create a single class, maybe called "Character", and make enemy and player both instances of it.

This next snippet will show the above changes. I introduced a few additional things, not major but to show some of the flexibility. The main thing is that a character can have a variable starting health, and a variable damage rate. So a rat monster might have health and damage values or 2 or 3, while a bigger creature will have stats similar to the player itself.

// Notice how I declare the variable only when necessary// This is a good practise.// Note that this isn't possible for "enemy", because the enemy// needs to retain its state (i.e. its health) between loops.int choice; cin >> choice;switch(choice) {case1: player.attack(enemy);break;case2:break;case3:break;default: cout << "Incorrect choice" << endl;break; }

This doesn't keep the exact logic of your program, but it is close enough. The implementation of the constructor and the "alive" member function are left as an excersize (translation: I'm lazy [smile]).