patterncppMinor
Artillery game C++ practice project
Viewed 0 times
artilleryprojectgamepractice
Problem
I've completed a program as a learning experience to make sure I've got the concepts down. I found a practice project online and decided to try it out. The program compiles and runs exactly how it should with no problems.
The program is a game called Artillery. You have to fire a cannon at one enemy at a time until you hit him. The player inputs the angle at which to shoot the cannon.
My question is this: Am I using correct coding practices? For instance, if I was on a team of people working on this, would my use of classes and objects be efficient?
Also, I would like any input on how to improve my code for this program or any tips for easier ways to write any part of the code. The code is as follows:
distang.h
game.h
highscore.h
startup.h
DistAng.cpp
```
#include
#include
#include "distang.h"
using namespace std;
// Calculates distance and angle of shot
double DistAng::distAng(double inAngle)
{
inAngle = (inAngle * pi) / 180; // Converts angle from std::cin into radians
// A bit of simple physics to calculate the distance of shot
The program is a game called Artillery. You have to fire a cannon at one enemy at a time until you hit him. The player inputs the angle at which to shoot the cannon.
My question is this: Am I using correct coding practices? For instance, if I was on a team of people working on this, would my use of classes and objects be efficient?
Also, I would like any input on how to improve my code for this program or any tips for easier ways to write any part of the code. The code is as follows:
distang.h
#pragma once
#ifndef DISTANG_H
#define DISTANG_H
// Calculates angle and distance of shot
class DistAng
{
public:
double distAng(double);
private:
double velocity = 200;
double gravity = 32.2;
double shotDistance;
double timeInAir;
double pi = 3.1415;
};
#endifgame.h
#pragma once
#ifndef GAME_H
#define GAME_H
// The main loop for playing the game
class Game
{
public:
void game();
private:
int killed = 0;
int enemyDistance = 0;
double inAngle;
char done;
int shots = 0;
bool killedEnemy = false;
};
#endifhighscore.h
#pragma once
#ifndef HIGHSCORE_H
#define HIGHSCORE_H
// Reads/saves high score from/to file
class HighScore
{
public:
void highScore(int);
private:
int intHighScore;
};
#endifstartup.h
#pragma once
#ifndef STARTUP_H
#define STARTUP_H
// Displays the game introduction
class Startup
{
public:
void startup();
private:
};
#endifDistAng.cpp
```
#include
#include
#include "distang.h"
using namespace std;
// Calculates distance and angle of shot
double DistAng::distAng(double inAngle)
{
inAngle = (inAngle * pi) / 180; // Converts angle from std::cin into radians
// A bit of simple physics to calculate the distance of shot
Solution
I see a number of things that may help you improve your code.
Don't abuse
Putting
Make sure you have all required
The code uses
$$ t = \frac{2 v_0 \sin \theta}{g} $$
$$ d = v_0 t \cos \theta $$
A little algebra can simplify this:
$$ d = \frac{2 v_0 \sin \theta}{g} v_0 \cos \theta $$
$$ d = \frac{2 v_0^2 \sin \theta \cos \theta}{g} $$
Since \$2 \sin\theta \cos\theta = \sin(2\theta)\$, this yields
$$ d = \frac{v_0^2 \sin(2 \theta)}{g} $$
Since we are converting from degrees to radians for the calculation, the code is this:
Don't abuse
using namespace stdPutting
using namespace std at the top of every program is a bad habit that you'd do well to avoid. It isn't necessarily wrong to use, but be aware of when you absolutely shouldn't do it (such as in header files).Make sure you have all required
#includesThe code uses
std::numeric_limits in Game.cpp but doesn't #include . Also, srand is used in multiple places, but ` is not included.
Use C++ style includes
Instead of including time.h you should instead use #include . The difference is in namespaces as you can read about in this question.
Use consistent naming
I would expect the header file for an object named HighScore that is defined in HighScore.cpp to be named HighScore.h but strangely, the header files all have lowercase versions of the corresponding implementation files.
Separate I/O from program logic
Right now most functions have both game logic and I/O. It's often better design to separate the two so that the game logic is independent of the I/O with the user. That way, if you wanted, say, to port the code to a GUI, only the I/O portions of the code would need to revised, and the game logic could stay identical.
Consider consolidating objects
The Startup class does only one simple thing which is to print a fixed string to std::cout. To me, it would make much more sense for that to simply be done in main (without any object) or perhaps as a member function of Game.
Consider refactoring object interfaces
The HighScore class has only a single function in its interface, which is a somewhat badly named function called highScore(int). The text description of the class says that it reads/saves the high score from/to a file. It may make more sense to actually have separate functions that are named read and save rather than one big function that does both. Even if you elect to keep the one big function, a name such as update would make more sense, and could use the read and save functions.
Don't duplicate important constants
The filename in HighScore.cpp is hardcoded right now, but worse than that, it's done in two completely indpendent places. Better would be to create a constant:
static const char *FILENAME = "HighScore.art";
Use string concatenation
The Startup.cpp file includes these lines:
cout << "Welcome to Artillery!\n\n";
cout << "You are in the middle of a war and being charged by thousands of enemies!\n";
cout << "You have one cannon, which you can shoot at any angle.\n";
cout << "You only have 10 cannonballs for this target.\n\n";
Each of those is a separate call to operator header.
Omit return 0
When a C++ program reaches the end of the compiler will automatically generate code to return 0, so there is no reason to put return 0; explicitly at the end of main.
Simplify the mathematics
The code really doesn't need the DistAng object. Instead, I would refactor that calculation as a simple member function of the Game object. Since I think Game is an entirely too generic name, I decided to call it CannonGame` instead. The formulas that you're using are these:$$ t = \frac{2 v_0 \sin \theta}{g} $$
$$ d = v_0 t \cos \theta $$
A little algebra can simplify this:
$$ d = \frac{2 v_0 \sin \theta}{g} v_0 \cos \theta $$
$$ d = \frac{2 v_0^2 \sin \theta \cos \theta}{g} $$
Since \$2 \sin\theta \cos\theta = \sin(2\theta)\$, this yields
$$ d = \frac{v_0^2 \sin(2 \theta)}{g} $$
Since we are converting from degrees to radians for the calculation, the code is this:
constexpr double velocity = 200.0;
constexpr double gravity = 32.2;
constexpr double pi = 3.1415926;
int CannonGame::cannonDistance(double degrees) const {
return velocity * velocity / gravity * std::sin(degrees * pi / 90);
}Code Snippets
static const char *FILENAME = "HighScore.art";cout << "Welcome to Artillery!\n\n";
cout << "You are in the middle of a war and being charged by thousands of enemies!\n";
cout << "You have one cannon, which you can shoot at any angle.\n";
cout << "You only have 10 cannonballs for this target.\n\n";std::cout << "Welcome to Artillery!\n\n"
"You are in the middle of a war and being charged by thousands of enemies!\n"
"You have one cannon, which you can shoot at any angle.\n"
"You only have 10 cannonballs for this target.\n\n";constexpr double velocity = 200.0;
constexpr double gravity = 32.2;
constexpr double pi = 3.1415926;
int CannonGame::cannonDistance(double degrees) const {
return velocity * velocity / gravity * std::sin(degrees * pi / 90);
}Context
StackExchange Code Review Q#101028, answer score: 9
Revisions (0)
No revisions yet.