Best way to modify a variable in a class

[Ch1156]

So whats the best way to modify a variable in a class? Below I have a version that does so through a pointer, but is it better to just make a function that modifies the variable instead?

#include <iostream>
#include <string>
#include <vector>
#include <memory>

class Attack
{
    public:
        Attack(const std::string& name, int power) : mName(name), mPower(power)
        {}

        std::string& GetName()
        {
            return mName;
        }

        const std::string GetName() const
        {
            return mName;
        }

        int GetPower() const
        {
            return mPower;
        }

    private:
        std::string mName{ "" };
        int mPower{ 0 };
};

class Character 
{
    public:
        Character(const std::string& name, int health)
            : mName(name), mHealth(health) {}

        std::vector<std::shared_ptr<Attack>> GetAttacks()
        {
            return mAttacks;
        }

        std::vector<std::shared_ptr<Attack>> GetAttacks() const
        {
            return mAttacks;
        }

        const std::string GetAttackNameByIndex(const int indexNumber) const
        {
            if(indexNumber >= 0 && indexNumber < static_cast<int>(mAttacks.size()))
            {
                return mAttacks[indexNumber]->GetName();
            }
            else
            {
                return "Error, index out of range\n";
            }
        }

        void AddAttack(const std::shared_ptr<Attack>& attack)
        {
            mAttacks.push_back(attack);
        }

    private:
        std::string mName;
        int mHealth;
        std::vector<std::shared_ptr<Attack>> mAttacks{};
};

int main() 
{
    Character player("Hero", 100);

    auto stomp = std::make_shared<Attack>("Stomp", 12);
    auto slash = std::make_shared<Attack>("Slash", 21);
    auto fireSpin = std::make_shared<Attack>("Fire Spin", 53);

    player.AddAttack(stomp);
    player.AddAttack(slash);
    player.AddAttack(fireSpin);

    player.GetAttacks()[0]->GetName() = "Earthquake";

    for (auto& attack : player.GetAttacks())
    {
        std::cout << attack->GetName() << '\n';
    }
    
    return 0;
}

[jonnin]

best for what?
your professor would probably say through a pair of "getter/setter" functions, one that gets the value, and one that sets it. Setting it may be optional if it is a computed result internally that you should not change outside the object's logic. Getters/Setters have two big advantages, one is OOP correctness, and the other is that you can apply validation here and reject attempts to set the variable to a bad value immediately, before it gets into the class logic and breaks anything important.

I often make my variables public and directly access them. This is fast and efficient (less bloated code in the classes, no function call for an assignment though these are usually removed by compiler anyway, other things), at the cost of OOP incorrectness. It suffers the reversed advantages above (they become disadvantages): you can't tie validation to it, and its not ideal for OOP designs. I use this set of rules: if none of my getters and setters DO anything other than get and set, then I do not use them at all with the exception that these functions are required (either by coding standard for the project, or to match other objects in the same code so they all work the same way, etc.. big picture reasons) they still get written. If ANY of the getters/setters DO something, then I write them all, so the object is consistent with itself.

and a pointer is usually RIGHT OUT. Do not do this unless you have a very compelling reason to provide it, and even then, think twice about it. A pointer to a class variable allows the user to bypass the object entirely and just directly access the values and directly change them, same as above (direct access) but with additional pointer related problems (eg if the user holds onto the pointer and it goes bad due to a deconstructed object). Pointer access adds an extra step to access data, both in code (bloat) and in the program execution, so it must be justified for both danger reasons as well as performance concerns.

[Ch1156]

So what do i do in this situation? I have some code that im working on and it has a character and attack class just like my example, and I have a function in the character class that adds an attack to the Attack vector for the player and enemies but when i go to battle it says that the enemy has no attacks, but if i use a pointer it works, so what should i do here? I will provide a link to my github since i think my code is too long to post here. Please excuse the messy code im in the middle of refactoring it.

https://github.com/ChayHawk/Fight-Arena/tree/development

[jonnin]

I will look at it when I can, but its a bug.
If you do it right, you can add an attack to the vector without having to resort to pointers. you should, actually, be able to do it via something like
vector<attack> foo;
... later
foo.push_back(attack(a,b)); // this should create a new attack with the ctor and push it onto the vector, and done.
you can also do it more verbosely via
attack atk(a,b);
foo.push_back(atk);

-----------
its pretty complicated in there. My advice is to validate it as you go. Eg when you add attacks to your orc right off in the beginning, stop there, exit the program after printing the attacks that your orc has now. If it has them, proceed until you can see where they were removed... use the run to cursor feature of visual studio to jump around and see where you lost them, just do a binary search off main. Also, do not load/save to file at first: see if it works without file operations at all, then see if the file is part of the problem after that. Saving a container to a file requires saving each item one by one and then pushing them back one by one when you load.

[Ch1156]

So I modified the example program to emulate what im doing in my other code more concisely. This is the issue i face:

EDIT: @jonnin I just saw your post, i'll try that out and see what happens.

#include <iostream>
#include <string>
#include <vector>

class Attack
{
public:
    Attack(const std::string& name, int power, int level) : mName(name), mPower(power), mLevel(level)
    {}

    std::string GetName() const
    {
        return mName;
    }

    int GetPower() const
    {
        return mPower;
    }

    void SetName(const std::string& name)
    {
        mName = name;
    }

    void IncrementLevel()
    {
        mLevel++;
    }

    int GetLevel() const
    {
        return mLevel;
    }

private:
    std::string mName{ "" };
    int mPower{ 0 };
    int mLevel{ 0 };
};

class Character
{
public:
    Character(const std::string& name, int health)
        : mName(name), mHealth(health) {}

    std::vector<Attack> GetAttacks() const
    {
        return mAttacks;
    }

    std::string GetAttackNameByIndex(int indexNumber) const
    {
        if (indexNumber >= 0 && indexNumber < static_cast<int>(mAttacks.size()))
        {
            return mAttacks[indexNumber].GetName();
        }
        else
        {
            return "Error, index out of range\n";
        }
    }

    void LearnAttack(const Attack& attack)
    {
        mAttacks.push_back(attack);
    }

    std::string GetName() const
    {
        return mName;
    }

private:
    std::string mName;
    int mHealth;
    std::vector<Attack> mAttacks{};
};

void ShowAttacks(std::vector<Character>& enemyList);

int main()
{
    Character player("Hero", 100);

    Character orc("Orc", 100);
    Character goblin("Goblin", 70);
    Character rat("Rat", 10);

    std::vector<Character> enemyList{ orc, goblin, rat };

    Attack punch("Punch", 7, 1);
    Attack scratch("Scratch", 3, 1);
    Attack orcSword("Orc Sword", 21, 1);
    Attack rifle("Rifle", 29, 1);
    Attack knife("Knife", 11, 1);

    orc.LearnAttack(punch);
    orc.LearnAttack(orcSword);

    goblin.LearnAttack(punch);
    goblin.LearnAttack(scratch);

    rat.LearnAttack(scratch);

    player.LearnAttack(knife);
    player.LearnAttack(rifle);
    player.LearnAttack(punch);

    player.GetAttacks()[0].IncrementLevel();

    ShowAttacks(enemyList);

    return 0;
}

void ShowAttacks(std::vector<Character>& enemyList)
{
    int index{ 0 };

    for (auto& enemy : enemyList)
    {
        std::cout << "Index Number: " << index << '\n';
        std::cout << "Enemy Attack List Size: " << enemy.GetAttacks().size() << '\n';

        if (enemy.GetAttacks().empty())
        {
            std::cout << enemy.GetName() << " has no attacks!\n\n";
        }
        else
        {
            std::cout << enemy.GetName() << " - LVL: " << enemy.GetAttacks()[index].GetLevel() << '\n';
            index++;
        }
    }
}

[Ch1156]

As for testing when it fails to add the attacks, it fails to add them to the vector right in main, i added this in my code from the github version, and it fails to find any, so the issue is in the Learn attack function not actually adding them to the vector.

int main()
{
    Character player("Hero", 100, 1, 0);

    Character goblin("Goblin", 30, 1, 0);
    Character orc("Orc", 42, 1, 0);
    Character dragon("Dragon", 500, 1, 0);

    std::vector<Character> enemyList{ goblin, orc, dragon };

    Attack slash("Slash", 4, 1, 1, 20);
    Attack scratch("Scratch", 1, 1, 25, 25);
    Attack fireBlast("Fire Blast", 12, 1, 15, 15);
    Attack stomp("Stomp", 7, 1, 25, 25);
    Attack orcSword("Orc Sword", 9, 1, 13, 13);
    Attack punch("Punch", 3, 1, 20, 20);
    Attack kick("Kick", 5, 1, 15,  15);
    Attack rifle("Assault Rifle", 15, 1, 30, 30);
    Attack handgun("Handgun", 10, 1, 8, 36);

    goblin.LearnAttack(stomp);
    goblin.LearnAttack(scratch);
    goblin.LearnAttack(punch);
    
    orc.LearnAttack(orcSword);
    orc.LearnAttack(punch);
    orc.LearnAttack(kick);

    dragon.LearnAttack(slash);
    dragon.LearnAttack(fireBlast);
    dragon.LearnAttack(stomp);

    player.LearnAttack(rifle);
    player.LearnAttack(handgun);

    for (auto& enemy : enemyList)
    {
        if (enemy.GetAttacks().empty())
        {
            std::cout << enemy.GetName() << " has no attacks!\n";
        }
    }

[Ch1156]

Ok so i came up with this, this works but idk if its bad to do, i used a reference instead

#include <iostream>
#include <string>
#include <vector>

class Attack
{
public:
    Attack(const std::string& name, int power, int level) : mName(name), mPower(power), mLevel(level)
    {}

    std::string GetName() const
    {
        return mName;
    }

    int GetPower() const
    {
        return mPower;
    }

    void SetName(const std::string& name)
    {
        mName = name;
    }

    void IncrementLevel()
    {
        mLevel++;
    }

    int GetLevel() const
    {
        return mLevel;
    }

private:
    std::string mName{ "" };
    int mPower{ 0 };
    int mLevel{ 0 };
};

class Character
{
public:
    Character(const std::string& name, int health)
        : mName(name), mHealth(health) {}

    std::vector<Attack> GetAttacks() const
    {
        return mAttacks;
    }

    std::string GetAttackNameByIndex(int indexNumber) const
    {
        if (indexNumber >= 0 && indexNumber < static_cast<int>(mAttacks.size()))
        {
            return mAttacks[indexNumber].GetName();
        }
        else
        {
            return "Error, index out of range\n";
        }
    }

    void LearnAttack(const Attack& attack)
    {
        mAttacks.push_back(attack);
    }

    std::string GetName() const
    {
        return mName;
    }

private:
    std::string mName;
    int mHealth;
    std::vector<Attack> mAttacks{};
};

void ShowAttacks(const std::vector<std::reference_wrapper<Character>>& enemyList);

int main()
{
    Character player("Hero", 100);

    Character orc("Orc", 100);
    Character goblin("Goblin", 70);
    Character rat("Rat", 10);

    std::vector<std::reference_wrapper<Character>> enemyList{ orc, goblin, rat };

    Attack punch("Punch", 7, 1);
    Attack scratch("Scratch", 3, 1);
    Attack orcSword("Orc Sword", 21, 1);
    Attack rifle("Rifle", 29, 1);
    Attack knife("Knife", 11, 1);

    orc.LearnAttack(punch);
    orc.LearnAttack(orcSword);

    goblin.LearnAttack(punch);
    goblin.LearnAttack(scratch);

    rat.LearnAttack(scratch);

    player.LearnAttack(knife);
    player.LearnAttack(rifle);
    player.LearnAttack(punch);

    player.GetAttacks()[0].IncrementLevel();

    ShowAttacks(enemyList);

    return 0;
}

void ShowAttacks(const std::vector<std::reference_wrapper<Character>>& enemyList)
{
    for (size_t index = 0; index < enemyList.size(); ++index)
    {
        const auto& enemy = enemyList[index].get();
        std::cout << "Index Number: " << index << '\n';
        std::cout << "Enemy Attack List Size: " << enemy.GetAttacks().size() << '\n';

        if (enemy.GetAttacks().empty())
        {
            std::cout << enemy.GetName() << " has no attacks!\n\n";
        }
        else
        {
            for (const auto& attack : enemy.GetAttacks())
            {
                std::cout << enemy.GetName() << " - " << attack.GetName() << " - LVL: " << attack.GetLevel() << '\n';
            }
        }
    }
}

[jonnin]

references are fine. Is it fixed then? I don't see what reference you mean ... I even looked through the git but its mostly pointer to not pointer fix (good). Where/what reference are you thinking fixed it?

[Ch1156]

In my example code above, I havent yet modified the github version, im just testing out using std::reference_wrapper before i go and modify my main code. So i can use reference_wrapper to do this then? or is there a better function to use other that reference_wrapper?

[seeplus]

shared_ptr/Unique_ptr are for expressing pointer ownership. If ownership is not an issue (as here) then you can just pass a raw pointer or have a vector of raw pointers (providing the object to which the pointer points is valid).

Consider:

#include <iostream>
#include <string>
#include <vector>

class Attack {
public:
	Attack(const std::string& name, int power, int level) : mName(name), mPower(power), mLevel(level) {}

	std::string GetName() const {
		return mName;
	}

	int GetPower() const {
		return mPower;
	}

	void SetName(const std::string& name) {
		mName = name;
	}

	void IncrementLevel() {
		++mLevel;
	}

	int GetLevel() const {
		return mLevel;
	}

private:
	std::string mName;
	int mPower { };
	int mLevel { };
};

using Attacks = std::vector<Attack*>;

class Character {
public:
	Character(const std::string& name, int health)
		: mName(name), mHealth(health) {}

	Attacks GetAttacks() const {
		return mAttacks;
	}

	std::string GetAttackNameByIndex(size_t indexNumber) const {
		if (indexNumber < mAttacks.size())
			return mAttacks[indexNumber]->GetName();

		return "Error, index out of range\n";
	}

	void LearnAttack(Attack* attack) {
		mAttacks.push_back(attack);
	}

	std::string GetName() const {
		return mName;
	}

private:
	std::string mName;
	int mHealth;
	Attacks mAttacks;
};

using Characters = std::vector<Character*>;

void ShowAttacks(const Characters& enemyList);

int main() {
	Character player("Hero", 100);
	Character orc("Orc", 100);
	Character goblin("Goblin", 70);
	Character rat("Rat", 10);

	Characters enemyList{ &orc, &goblin, &rat, &player };

	Attack punch("Punch", 7, 1);
	Attack scratch("Scratch", 3, 1);
	Attack orcSword("Orc Sword", 21, 1);
	Attack rifle("Rifle", 29, 1);
	Attack knife("Knife", 11, 1);

	orc.LearnAttack(&punch);
	orc.LearnAttack(&orcSword);

	goblin.LearnAttack(&punch);
	goblin.LearnAttack(&scratch);

	rat.LearnAttack(&scratch);

	player.LearnAttack(&knife);
	player.LearnAttack(&rifle);
	player.LearnAttack(&punch);

	player.GetAttacks()[0]->IncrementLevel();

	ShowAttacks(enemyList);
}

void ShowAttacks(const Characters& enemyList) {
	for (size_t index {}; index < enemyList.size(); ++index) {
		const auto& enemy { enemyList[index] };

		std::cout << "Index Number: " << index << '\n';
		std::cout << "Enemy Attack List Size: " << enemy->GetAttacks().size() << '\n';

		if (enemy->GetAttacks().empty())
			std::cout << enemy->GetName() << " has no attacks!\n\n";
		else
			for (const auto& attack : enemy->GetAttacks())
				std::cout << enemy->GetName() << " - " << attack->GetName() << " - LVL: " << attack->GetLevel() << '\n';
	}
}

[jonnin]

I think he needs copies.
If you use a pointer or a reference to an attack for rat, and increment its power level, that shouldn't increment it for a goblin (?). They seem to be more than lookups. Technically he could use setname to change rat's punch to 'bite' too...


If they ARE just lookups, and the user does not provide them (that is, at the end of your game development, your attacks are just a fixed piece of data) then get rid of the class entirely and make a fixed or read from file once container of them and just reference it -> in that case an index, pointer, or reference are all equally appropriate as you see fit. A named index via an enum is fine, eg attack[punch];

[Ch1156]

So I'm having a little trouble understanding just when to use pointers and how to navigate using inheritance and polymorphism on a deeper level, i understand the very basics but I don't fully understand how it all works, but I know pointers are needed for polymorphism. So what would using a pointer do here? I man it gets the actual object right? but you said if you use a pointer or reference and increment attack it can increment that value for other enemy characters too? I'm very confused. I know pointers and references allow you to just modify the actual value itself without copying it but idk, i don't seem to understand when its appropriate to use it.

[jonnin]

you are not using any polymorphism that I saw. I may have missed something, but lets table that for a moment.

lets say you have 3 attacks only, and it never changes. one way to allocate those to characters is to provide a pointer to the appropriate one for each character, eg a rat gets bite, a human gets punch, and an orc gets shoot, and then a snake is added and it also gets bite. This sort of approach means the 3 attacks are constants, and just assigned as-is. They can't be incremented, renamed, or otherwise changed because if you do it this way, and change bite to poisonous bite for the snake, the rat also has that change, because using a pointer ties the same copy of bite to both characters. Its a great way to save space and time and all if your attacks are constant, but it won't work if they need to be modified. So pointers here could be used to save creating unnecessary copies of constant objects.

another way to do it is that each character pushes back an attack that you create on the fly. This is a full, complete attack object each time, even if you have 10 animals that can all bite and claw, they still each get their own copy of it. Then you can have internal variables in the attack class that change, like the bite name change above, but here, because each one has its own copy, the change only affects snake. I think this is what you must do, because you have methods that modify the attack class's variables and they may not match all the characters needs. It uses more space and time to create full attack objects for each one instead of sharing constant ones, but if its the right thing to do, you have to do it.

do you understand that? Its the difference between this:
int attack = 3;
int *ratattack = &attack;
int *snakeattack = &attack;
*ratattack ++;
cout << *snakeattack; //its 4! changing rat changed snake too!

vs
int ratattack = 3;
int snakeattack = 3;
snakeattack++; //and so on, but here clearly changes to snake don't change rat.

for this sort of concept, references and pointers are almost identical. So changing a reference to a bite is just like changing it via a pointer, and sharing a reference to bite across 2 characters with a reference is the same as sharing it with a pointer. Shared is shared... change one, and the other one is also changed.

---------------------------------
are you confused by pointers generally?

[Ch1156]

Yeah i dont have any polymorphism at the moment, i was just saying that i know when i do get to that point, i will need pointers so i should learn this stuff deeply now while i can.

I believe I do understand a bit clearer now. So basically if i use a pointer to the attack, any changes a character makes to attack, say the name or level, will be changed for ALL characters, not just rat for example. so if i want each character to have different attack powers, say for example I had a Scratch attack, and the power was different depending on the size of the enemy, so for a raptor, scratch would do more damage than a rats scratch attack, so if i use a pointer here, rat and the raptor will have the same power level attack right? and any changes i make to scratch from there on will be visible in that attack for both enemies correct?

And this will happen also if i use a reference too correct?

As for the differences between pointers and references, references cannot be reseated once they are assigned right? and pointers can? isnt that the only big difference? and references HAVE to have a value upon creation, whereas pointer can be empty?

Yeah how pointers work and when to use them is really confusing and i have a vague understanding of them.

[seeplus]

I think part of the problem is that the design hasn't been fully thought-through before coding. IMO you need to first map out the game and it's design and how everything fits together and is supposed to work before starting the coding.

You probably want that a character 'has an' attack so character has it's own copy of attack. Possibly something like:

#include <iostream>
#include <string>
#include <vector>
#include <iterator>
#include <exception>

class Attack {
public:
	Attack(const std::string& name, int power, int level) : mName(name), mPower(power), mLevel(level) {}

	std::string GetName() const {
		return mName;
	}

	int GetPower() const {
		return mPower;
	}

	void SetName(const std::string& name) {
		mName = name;
	}

	void IncrementLevel() {
		++mLevel;
	}

	int GetLevel() const {
		return mLevel;
	}

private:
	std::string mName;
	int mPower { };
	int mLevel { };
};

using Attacks = std::vector<Attack>;

class Character {
public:
	Character(const std::string& name, int health)
		: mName(name), mHealth(health) {}

	Attack& GetAttackByIndex(size_t indexNumber) {
		if (indexNumber < mAttacks.size())
			return mAttacks[indexNumber];

		throw std::runtime_error("Index error");
	}

	size_t numAttacks() const {
		return mAttacks.size();
	}

	void LearnAttack(Attack attack) {
		mAttacks.emplace_back(std::move(attack));
	}

	std::string GetName() const {
		return mName;
	}

private:
	std::string mName;
	int mHealth;
	Attacks mAttacks;
};

using Characters = std::vector<Character*>;

void ShowAttacks(const Characters& enemyList);

int main() {
	Character player("Hero", 100);
	Character orc("Orc", 100);
	Character goblin("Goblin", 70);
	Character rat("Rat", 10);

	Characters enemyList{ &orc, &goblin, &rat, &player };

	// Default values.
	const Attack punch("Punch", 7, 1);
	const Attack scratch("Scratch", 3, 1);
	const Attack orcSword("Orc Sword", 21, 1);
	const Attack rifle("Rifle", 29, 1);
	const Attack knife("Knife", 11, 1);

	orc.LearnAttack(punch);
	orc.LearnAttack(orcSword);

	goblin.LearnAttack(punch);
	goblin.LearnAttack(scratch);

	rat.LearnAttack(scratch);

	player.LearnAttack(knife);
	player.LearnAttack(rifle);
	player.LearnAttack(punch);

	player.GetAttackByIndex(0).IncrementLevel();

	ShowAttacks(enemyList);
}

void ShowAttacks(const Characters& enemyList) {
	for (size_t index {}; index < enemyList.size(); ++index) {
		const auto& enemy { enemyList[index] };

		std::cout << "Index Number: " << index << '\n';
		std::cout << "Enemy Attack List Size: " << enemy->numAttacks() << '\n';

		if (enemy->numAttacks() == 0)
			std::cout << enemy->GetName() << " has no attacks!\n\n";
		else
			for (size_t i {}; i < enemy->numAttacks(); ++i) {
				const auto& attck { enemy->GetAttackByIndex(i) };

				std::cout << enemy->GetName() << " - " << attck.GetName() << " - LVL: " << attck.GetLevel() << '\n';
			}
	}
}

[Ch1156]

Certainly, that is an issue as well, but I would like to learn how to effectively use pointers as its something ive been putting off for 14 years haha. What would be your approach to this situation? I just did it this way simply because I dont know of any others.

[seeplus]

Have a look at:
https://www.learncpp.com/cpp-tutorial/introduction-to-pointers/

PS. See my revised code above.

[jonnin]

yes, read that ^^
but the 'quick' answer to your question is that pointers can be used for dynamic memory management, while references are tied to an existing item and can't be used for that.

there are also other oddities like a pointer can point to a function, but I don't think a reference can do that.

[seeplus]

and a pointer can have a null value (nullptr) whereby a reference can't.

[Ch1156]

Ok, so tell me if im right. I believe that using a pointer in this context is necessary to prevent copying, because simpler types are modified directly, like int, string, etc, however complex types like a class are copied into vectors, so if we want to modify them then they need to be a vector of pointers to (in my case) Character objects. So when i add characters to the vector, its actually modifying the Character objects in the vector itself, allowing me to do things like level up attacks, or change a name etc, correct?

Its just hard for me to visualize the actual mechanism behind it. normally in my code i can visualize how things work under the hood a bit, like function calls, how containers work, like adding/removing elements, the order in which variables are created and destroyed, the flow of if statements etc. but its hard for me to visually grasp whats happening here, and i think thats whats making me struggle a bit, i need to be able to see whats going on in my head, like have some kind of mental image of the process.