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

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?? – πάντα ῥεῖ Apr 4 '17 at 22:04
• Other programming languages require it so it's more of a habit and formatting style. – Mvpg Apr 4 '17 at 22:13
• Have you tried to compile this, and does it compile, I see many things that should be flagged by the compiler as errors. – pacmaninbw Apr 4 '17 at 22:15
• I have compiled it. – Mvpg Apr 4 '17 at 22:18
• @Mvpg "I have compiled it." Sure? – πάντα ῥεῖ Apr 4 '17 at 23:00

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 {
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;
};

• I'd suggest noting that rand() isn't reliable and should be avoided. – Kodnot Apr 10 '17 at 16:11
• @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. – Martin York Apr 10 '17 at 17:14
• 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) – Kodnot Apr 12 '17 at 11:08