0

I have a Spieler class and a Verein class with a vector of Spieler members. Now if I change something of the Players like the Staerke(german for strength) by using a function of this class in the player class it does not automatically change the value for this player. Here is the code:

#include <vector>
#include<iostream>
#include <string>
using namespace std;

class Spieler
{
    public:


        void setinformation(int a, string b, string c, int d)
        {
            ID = a;
            Vorname = b;
            Nachname = c;
            Staerke = d;
        }
        void getinformation()
        {
            cout << "ID: " << ID << endl;
            cout << "Vorname: " << Vorname << endl;
            cout << "Nachname: " << Nachname << endl;
            cout << "Staerke: " << Staerke << endl << endl;
        }
        void setStaerke(int x)
        {
            Staerke = x;
        }

        int getStaerke()
        {
            return Staerke;
        }

    private:

        string Vorname, Nachname;
        int Staerke, ID;

};

class Verein
{
    public:

        void setSpielerListe(vector<Spieler> x)
        {
            Spielerliste = x;
        }

        vector<Spieler> getSpielerListe()
        {
            return Spielerliste;
        }
        string getVereinsName()
        {
            return VereinsName;
        }

        int getVereinsID() const
        {
            return VereinsID;
        }

        void setVereinsID(int x)
        {
            VereinsID = x;
        }
        int getGesamtstaerke()
        {
            Gesamtstaerke = 0;
            vector<Spieler> b;
            b = getSpielerListe();
            for (size_t i = 0; i < b.size(); i++)
            {
                Gesamtstaerke = Gesamtstaerke + b[i].getStaerke();
            }
            return Gesamtstaerke;
        }

        void Vereinsinformationen()
        {
            vector<Spieler> list;
            int id;
            string vereinsname;
            int gesamtstaerke;
            id = getVereinsID();
            vereinsname = getVereinsName();
            gesamtstaerke = getGesamtstaerke();
            list = getSpielerListe();
            cout << "VereinsID: " << id << endl;
            cout << "Vereinsname: " << vereinsname << endl;
            cout << "Gesamstaerke: " << gesamtstaerke << endl << endl;
            cout << "Spieler: " << endl;
            for (size_t i = 0; i < list.size(); i++)
                list[i].getinformation();
        }

    private:
        vector<Spieler> Spielerliste;
        int VereinsID, Gesamtstaerke;
        string VereinsName;

};

vector<Spieler> spieler;
int main()
{
    Spieler Spieler1;
    Spieler1.setinformation(0, "Peter", "Pan", 10);
    spieler.emplace_back(Spieler1);
    Verein Team1;
    Team1.setSpielerListe(spieler);
    Spieler1.setStaerke(20);
    Team1.Vereinsinformationen();
    cin.get();           

    return 0;
}

I'm really new into c++ and programming so the code might be terrible. Guess it has to do with pointers, I'm really not into the concept of storing data in c++, try to get it by trial & error; So how to change the Staerke in a way that it is changed in the Teams Playerlist too?

Hulk
  • 6,399
  • 1
  • 30
  • 52
jimbo999
  • 80
  • 7
  • I might be wrong, but it's better to turn setinformation() into a constructor, i.e. Spieler(12, Klaus, Heinz, 50); because a constructor is essentially an information setter. That;s not the answer to your question, by the way, just advice. – Leo Mar 11 '14 at 11:44
  • Side notes: a very common convention is that variable names should start with a lowercase letter, and only types with an uppercase letter. Also, `using namespace std;` [is considered bad practice](http://stackoverflow.com/q/1452721/2513200). And using only English variable names and comments makes it easier to communicate globally (like you just noticed). – Hulk Mar 11 '14 at 11:49
  • 1
    I would really recommend using english as a programming language. Your language structures are english too, so why translate domain logic? Excel translated language structures for every language and it is really really awful. – Samuel Mar 11 '14 at 11:49
  • You mean i should use english variables instead of german ? – jimbo999 Mar 11 '14 at 11:53
  • @jimbo999 Yes, you should get used to using English variable names (and comments). It adds to the readability of your code, and if the names are well chosen, they communicate your intentions, which makes it easier for other people to spot issues. (You'll notice how useful that is once you've tried to read something that has only variables in a language you don't speak or can't even read, e.g. Chinese or Arabic letters) – Hulk Mar 11 '14 at 11:59

2 Answers2

2

The problem is you are storing full object in the vector and not pointers. When you run this line:

spieler.emplace_back(Spieler1);

a copy of Spieler1 is made and put in the vector. So modifying it in the main will have no effect in the vector. Also not that you are copying the vector when setting in Verein class.

You should use pointer if this is what you are after or better yet have a function to modify strength from Verein class taking its id and new strength as parameters might be a good idea. Something like this:

void setStaerke(int id, int x)
{
    vector<Spieler>::iterator it = Spielerliste.begin();
    while (it != Spielerliste.end())
    {
        if ((*it).GetId() == id)
        {
            (*it).setStaerke(x);
            break;
        }
    }
}

If you have access to C++11, it could be made more elegantly.

Eric Fortin
  • 7,533
  • 2
  • 25
  • 33
  • thx but how to make it with pointers ? As i said, haven't got the idea of them up to now. Tried spieler.emplace_back(&Spieler1) but that does not work.. – jimbo999 Mar 11 '14 at 11:50
  • You should really use smart_ptr for that but like I said, it would most likely better design to modify the strength from the Verein class instead and let it manage memory. – Eric Fortin Mar 11 '14 at 11:51
  • You should try to avoid using pointers for this task at all costs. It only makes things complicated. – moooeeeep Mar 11 '14 at 12:00
  • Why should i avoid them ? This is only a part of the project, at the end there a some thousands of players and some hundred of teams ? Thought there could arise some memory problems or something like this. @Eric Fortin: Thx, works like i wanted it to – jimbo999 Mar 11 '14 at 12:07
  • It's always a matter of trade off in simplicity, performance, etc. If you need to modify player objects after it has been added to the vector, you need to use pointers. You should although avoid to use raw pointers and use smart_ptr or unique_ptr to manage memory depending on your use case. – Eric Fortin Mar 11 '14 at 12:14
1

Hereby you pass and store a copy from the vector into the object:

Team1.setSpielerListe(spieler);

Therefore changes to the original vector and the contained objects will not affect the member.

Further, I don't have much experience with emplace_back, but the more usual way to append an object to a std::vector would also append a copy:

spieler.push_back(Spieler1);

Therefore changes to the original object would not affect the object you've appended to the container.

Make sure you better understand when objects are copied.

For reference:

Community
  • 1
  • 1
moooeeeep
  • 31,622
  • 22
  • 98
  • 187