patterncppMinor
Text-based RPG game in C++
Viewed 0 times
textgamebasedrpg
Problem
This is a text-based RPG game I've made in C++ and would love some review on it. I am a 100% complete newbie with maybe 2 weeks of C++ experience so I'd love some lessons on what I did wrong and how I can improve! I mainly just use a lot of switch statements and
If there is anything wrong or things that could change and improve I'd very much appreciate if you could walk me through what was wrong, why it was wrong and how to prevent it from happening again. I really don't know a lot of C++ terms so ELIF would be awesome.
Here's my simple main.cpp. I think that's called a constructor and I'm using it to call my MeadowGiant.cpp down the road.
MeadowGiant.h
MeadowGiant.cpp
```
#include "MeadowGiant.h"
#include
#include
#include
#include
#include
MeadowGiant::MeadowGiant()
{
int trail, caveEnter;
int fight1;
int loot;
int goblinAttack, goblinHP, HP, attack, action1, ability;
goblinAttack = 5;
goblinHP = 10;
HP = 100;
std::cout > trail;
if(trail==1)
{
std::cout > caveEnter;
if(caveEnter==1)
{
std::cout > fight1;
goblinFight:
loot = rand() % 4 + 1;
srand(time(NULL));
if(fight1==1)
{
std::cout > action1;
}
if(goblinHP<=0)
{
std::cout << "You have slain the goblin!\n"
<< "You head towards the chest and take the spoils\n"
<< "of battle!\n";
switch(loot)
{
case 1: std::cout << "You find a bastard sword!\n";
attack = attack + 7;
goto exitCave;
case 2: std::cout << "You find an Enchanted Staff!\n";
attack = attack + 10;
if statements.If there is anything wrong or things that could change and improve I'd very much appreciate if you could walk me through what was wrong, why it was wrong and how to prevent it from happening again. I really don't know a lot of C++ terms so ELIF would be awesome.
Here's my simple main.cpp. I think that's called a constructor and I'm using it to call my MeadowGiant.cpp down the road.
#include
#include
#include "MeadowGiant.h"
int main()
{
MeadowGiant obj1;
}MeadowGiant.h
#define MEADOWGIANT_H
class MeadowGiant
{
public:
MeadowGiant();
protected:
};
#endifMeadowGiant.cpp
```
#include "MeadowGiant.h"
#include
#include
#include
#include
#include
MeadowGiant::MeadowGiant()
{
int trail, caveEnter;
int fight1;
int loot;
int goblinAttack, goblinHP, HP, attack, action1, ability;
goblinAttack = 5;
goblinHP = 10;
HP = 100;
std::cout > trail;
if(trail==1)
{
std::cout > caveEnter;
if(caveEnter==1)
{
std::cout > fight1;
goblinFight:
loot = rand() % 4 + 1;
srand(time(NULL));
if(fight1==1)
{
std::cout > action1;
}
if(goblinHP<=0)
{
std::cout << "You have slain the goblin!\n"
<< "You head towards the chest and take the spoils\n"
<< "of battle!\n";
switch(loot)
{
case 1: std::cout << "You find a bastard sword!\n";
attack = attack + 7;
goto exitCave;
case 2: std::cout << "You find an Enchanted Staff!\n";
attack = attack + 10;
Solution
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
Use only necessary
The
Don't reseed the random number generator more than once
The program currently calls
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
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
#includesThe
#include 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 ` 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 . 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.Code Snippets
std::cout << "You have slain the goblin!\n"
<< "You head towards the chest and take the spoils\n"
<< "of battle!\n";std::cout << "You have slain the goblin!\n"
"You head towards the chest and take the spoils\n"
"of battle!\n";for (goblinHP = 10; goblinHP > 0; ) {
// code to implement goblin attack goes here
}Context
StackExchange Code Review Q#103740, answer score: 8
Revisions (0)
No revisions yet.