Skip to main content
Code Review

Return to Revisions

2 of 2
replaced http://stackoverflow.com/ with https://stackoverflow.com/

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

Fix typos

There were so many typos in this code that I had to do quite a number of fixes before the code would even compile. I'd recommend composing the question offline, using a text editor that changes tabs to spaces. Then cut and paste the whole thing into CodeReview. It will save many people lots of time, including and especially you!

Don't put everything into the constructor

The MeadowGiant class has all of its code in the constructor. That's not very good design. Instead, it would be better to instantiate one or more of them and then use member functions to manipulate the object.

Use only necessary #includes

The #include <stdio.h> line is not necessary and can be safely removed.

Don't reseed the random number generator more than once

The program currently calls srand after every call to rand(). This is really neither necessary nor advisable. Instead, just call it once when the program begins and then continue to use rand() to get random numbers. Better yet, see the next suggestion.

Consider using a better random number generator

If you are using a compiler that supports at least C++11, consider using a better random number generator. In particular, instead of rand, you might want to look at std::uniform_real_distribution and friends in the <random> header.

Use constant string concatenation

This code currently has a number of instances that look like this (or would if the typos were fixed):

std::cout << "You have slain the goblin!\n"
 << "You head towards the chest and take the spoils\n"
 << "of battle!\n";

This calls the << operator three times. Instead, you could write this:

std::cout << "You have slain the goblin!\n"
 "You head towards the chest and take the spoils\n"
 "of battle!\n";

This only calls << once. The compiler automatically concatenates the string literals together.

Avoid using goto

Having a proliferation of goto statements is usually a sign of bad design. Better would be to eliminate them entirely -- it makes the code easier to follow and less error-prone. In this code, it's probable that you could use a loop instead:

for (goblinHP = 10; goblinHP > 0; ) {
 // code to implement goblin attack goes here
}

Use C++-style includes

Instead of including stdlib.h you should instead use #include <cstdlib>. The difference is in namespaces as you can read about in this question.

Eliminate unused variables

Unused variables are a sign of poor quality code, and you don't want to write poor quality code. In this code, ability is unused. Your compiler is smart enough to tell you about this if you ask it nicely.

Edward
  • 67.2k
  • 4
  • 120
  • 284
default

AltStyle によって変換されたページ (->オリジナル) /