Passing C++ objects by reference for participants in a simulated battle

Question

I just started watching C++ videos couple of days ago. I tried taking everything I learned about classes and passing by references vs values. The main question I what would like to know is this the correct way to change the values of private class members or is there a more 'professional' way of doing it. (Or standard).

Entity.cpp:

void Entity::playerAttack(Entity &Player, Entity &Enemy) {
 cout << "\nPlayer is Attacking!" << endl;
 cout << "Enemy's health was at: " << Enemy.getHealth() << endl;
 totalHealth = Enemy.getHealth() - Player.getDamage();
 Enemy.setHealth(totalHealth);
 if (Enemy.getHealth() <= 0) {
 Enemy.setHealth(0);
 cout << "\nEnemies is 0 or below" << endl;
 cout << "Players health is at: " << Player.getHealth() << endl;
 }
 else {
 cout << "Enemies health is now at: " << Enemy.getHealth() << endl;
 }
}
void Entity::setHealth(float x) {
 health = x;
}
void Entity::setDamage(float x) {
 damage = x;
}
float Entity::getDamage() {
 return (damage);
}
float Entity::getHealth() {
 return (health);
}

Entity.h:

#pragma once
class Entity
{
public:
 void setHealth(float x);
 void setDamage(float x);
 void playerAttack(Entity &Player, Entity &Enemy);
 float getHealth();
 float getDamage();
 Entity();
 ~Entity();
private:
 float health;
 float damage;
 float healAbility;
 float totalHealth;
};

source.cpp:

Entity constructE(float damage, float health, float regen) {
 Entity Entity;
 Entity.setDamage(damage);
 Entity.setHealth(health);
 Entity.setRegen(regen);
 return (Entity);
}
int main() {
 Entity Player = constructE(10, 100, 10);
 Entity Monster = constructE(5, 20, 5);
 Player.playerAttack(Player, Monster);
}

• πάντα ῥεῖ: return (damage); What's the purpose of the parens??
• Mvpg: Other programming languages require it so it's more of a habit and formatting style.
• pacmaninbw: Have you tried to compile this, and does it compile, I see many things that should be flagged by the compiler as errors.
• Mvpg: I have compiled it.
• πάντα ῥεῖ: @Mvpg "I have compiled it." Sure?

Answer 1

I have a couple of issues with how you are doing this (it is very Java like not C++ like).

Constructors

After the constructor finishes your object should be in a valid consistent state. Having to call multiple "setters" to finish initialization is a big red flag.

Entity constructE(float damage, float health, float regen) {
 Entity Entity;
 Entity.setDamage(damage);
 Entity.setHealth(health);
 Entity.setRegen(regen);
 return (Entity);
}

Why not just have a "constructor" that takes these arguments.

Entity Player = constructE(10, 100, 10);
// Should look like:
Entity Player(10, 100, 10);

Passing an object to itself?

Player.playerAttack(Player, Monster);

You are passing player as an argument? As you are calling a method on the object "player" you already have access to the player (via this). So there is no need to pass the player here.

player.attack(monster);

Identifier conventions.

One thing I notice about your code is that everything starts with a capitol letter. This makes it hard to tell the difference between types and objects. One of the most important things in C++ is type.

So the usual convention is that "User Defined Types" have an initial capitol letter. While objects (variables) have an initial lower case letter.

player.attack(monster);
^ Object ^ Object

While:

Player.attack(monster);
| ^ object
^ Looks like a type.
 So attack looks likes a static member function (ie a class method).

Getters/Setters

Getters and Setters break encapsulation. They expose the internal implementation details of the class. If you see the pattern

 auto x = object.getValue();
 auto y = performSomeAction(x);
 object.setValue(y);

Then this is usually an indication that you have not designed your class well as you expose internal details to calculate a value. A better design would be to provide a method that performs an ACTION on the class.

 object.someAction(); // internally it can update its own state.

Re-Design

class Entity
{
public:
 Entity(int damage, int helath, int regen)
 : damage(damage)
 , health(health)
 , healAbility(regen)
 {}
 // Destructor not needed.
 // Check if an entity is alive.
 operator bool() const
 {
 return health > 0;
 }
private:
 // Internal use only by attack()
 int weaponDamage() const
 {
 return myRandomNumber() % 10; // Damage in the range 0..9
 }
 // Take damage
 Entity& takeDamage(int amount)
 {
 // Can not go below zero.
 health = max(0, health - amount);
 return *this;
 }
public:
 // Attack an Enemy
 void attack(Entity &enemy)
 {
 std::cout << "Player: " << *this
 << "Attacking: " << enemy;
 // Damage the enemy and check if he is alive in one line.
 if (enemy.takeDamage(weaponDamage()) {
 // Enemy Alive
 cout << "E. Status: " << enemy;
 }
 else {
 // Enemy Dead
 cout << "Enemies is Dead" << "\n";
 cout << "P: Status: " << *this;
 }
 }
 virtual std::ostream& print(std::ostream& str) const
 {
 return str << "health: " << health 
 << " damage: " << damage
 << " healAbility: " << healAbility
 << "\n";
 }
 friend std::ostream& operator<<(std::ostream& str, Entity const& data)
 {
 return data.print(str);
 }
private:
private:
 float health;
 float damage;
 float healAbility;
};

• Kodnot: I'd suggest noting that rand() isn't reliable and should be avoided.
• Loki Astari: @Kodnot: So would I (if we wanted soething really random or cryptographically secure). But that's not part of the point I was trying to make and distracts for the correct discussion.
• Kodnot: Although I do agree that proper randomness isn't really important here, I still believe that the subject should be informed. Ignoring such (small) mistakes / bad pieces of code can form bad habits, which are harder to get rid of later on. There is no need to give an in-depth explanation of the drawbacks of rand(), but I don't think that simply informing the subject that there are better alternatives would distract him from the correct discussion. (*Although now that I look at the code again, the subject didn't use randomness at all, so all this ranting of mine is pointless :P)