C++

Forum

 
Editing member variables of objects in a vector

Jimbot (29)

Hello,

I have some code which adds a bunch of objects to a vector. I then wish to retrieve a certain object from the vector and edit its member variable.

At the moment it doesn't work - I think this is because I am only editing a local copy of the member variable (which is then destroyed when it goes out of scope), and I believe to overcome this I need to use a reference to my element but I'm not really sure......any guidance would be greatly appreciated.....

Here is my code:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
 

#include <iostream>
#include <vector>

using namespace std;

class Element
{
	public:
		Element();
		int GetValue();
		void SetValue(int NewValue);
	private:
		int MyValue;
};

Element::Element()
{
	MyValue = 5;
}

int Element::GetValue()
{
	return MyValue;	
}

void Element::SetValue(int NewValue)
{
	MyValue = NewValue;
}

class Collection
{
	public:
		Element GetElement(int ElementNumber);
		void AddElement(Element NewElement);
	private:
		vector<Element> AllElements;
};

void Collection::AddElement(Element NewElement)
{
	AllElements.push_back(NewElement);
}

Element Collection::GetElement(int ElementNumber)
{
	return AllElements[ElementNumber];
}


int main()
{
	Collection MyCollection;
	Element CurrentElement;	
	int i;

	cout << "Creating new elements and adding them to the collection...." << endl;

	for(i = 0; i < 5; i++)
	{
		Element NewElement;
		MyCollection.AddElement(NewElement);
	}

	CurrentElement = MyCollection.GetElement(2);
	cout << "The value inside element 3 is: " << CurrentElement.GetValue() << endl;

	cout << "Trying to change the value inside element 3 to 7...." << endl;
	CurrentElement.SetValue(7);

	cout << "The value inside element 3 is: " << CurrentElement.GetValue() << endl;

	return 0;
}


Thanks,

Jimbot
closed account (D80DSL3A)
I think you are just changing the value in the copy of the Element returned by GetElement().
You wish to change the value of the Element stored in the vector?
You could do it with MyCollection.AllElements[2].SetValue(7); if the vector were a public member of Collection. It's private though so having the GetElement() return a reference does seem necessary.

Why make AllElements private if you want to be able to alter values stored in it from outside the class methods? This defeats the purpose of making it private.
dangrr888 (80)
Hi Jimbot,

here is a solution for your problem:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
 
#include <iostream>
#include <vector>
#include <cstddef>

using namespace std;

class Element
{
public:
  Element()
    : MyValue(5) {}

  int GetValue() { return MyValue; }
  void SetValue(int NewValue) {	MyValue	= NewValue; }

private:
  int MyValue;
};

class Collection
{
public:
  Element& GetElement(vector<Element>::size_type ElementNumber);
  void AddElement(const Element& NewElement);

private:
  vector<Element> AllElements;
};

void Collection::AddElement(const Element& NewElement)
{
  AllElements.push_back(NewElement);
}

Element& Collection::GetElement(vector<Element>::size_type ElementNumber)
{
  return AllElements[ElementNumber];
}


int main()
{
  Collection MyCollection;

  cout << "Creating new elements and adding them to the collection...." << endl;

  for(vector<Element>::size_type i = 0; i < 5; i++)
    MyCollection.AddElement(Element());

  Element CurrentElement = MyCollection.GetElement(2);
  cout << "The value inside element 3 is: " << CurrentElement.GetValue() << endl;

  cout << "Trying to change the value inside element 3 to 7...." << endl;
  CurrentElement.SetValue(7);

  cout << "The value inside element 3 is: " << CurrentElement.GetValue() << endl;

  return 0;
}


the output is now:


Creating new elements and adding them to the collection....
The value inside element 3 is: 5
Trying to change the value inside element 3 to 7....
The value inside element 3 is: 7


@fun2code
Why make AllElements private if you want to be able to alter values stored in it from outside the class methods? This defeats the purpose of making it private.


I agree, the existence of Element::SetValue makes Collection::AllElements completely accessible, despite the fact that it is supposed to be private.

I would suggest removing Element::SetValue and impose the responsibility of the client to correctly construct their Collection objects and the Elements they append to them.

Perhaps with the option of initialising a Collection from a correctly formatted data file where individual Elements can be constructed within a new Collection::Collection(std::istream&) constructor that uses a new member function Collection::read(std::istream&) to construct new Elements from the data file and append them to the Collection object.

Good luck!
closed account (D80DSL3A)
Yes. Also, having an Element class constructor which takes an int value wouldn't hurt either.
Element(int ival) : MyValue(ival) {}
The values could then be assigned as desired in the first place, eliminating the need to re assign them after construction.
1
2
3
4
5
6
 
int inVal;
for(vector<Element>::size_type i = 0; i < 5; i++)
{
	cout << "Enter a value: "; cin >> inVal;
	MyCollection.AddElement(Element(inVal));
}

Jimbot (29)

Thank you both very much for your help!

I am very interested in the different design approaches you suggest.

I have some questions about your answers but I am going to review and digest the answers you have given me before I post them......

Thanks again,

Jimbot

Jimbot (29)

Hello again,

Thanks again for your help.

In answer to this question,

Why make AllElements private if you want to be able to alter values stored in it from outside the class methods?


This was because I misunderstood the way classes handle data. I thought that because I had made all my element methods which can talk to private members of the element class, that when I added objects of this class to a vector in another array then they could still be accessed and changed as usual. I didn't appreciate that once I add them to a private member of another class then they also become private. (I hope that makes some sense)

As for
I would suggest removing Element::SetValue
I wish to still keep this as I want to be able to edit element data during run time, and although I could add this functionality to the Collection class I think the code would lose some of its 'object orientated-ness'

I wish to use the structure as part of a game where I would have a player class (the element class in the above example) and a Game class which would hold a list of players (as player objects in a vector) (the Collection class in the above example).

Members of the player class need to be able to be edited as the game is played so I will need player member functions.

I think the structure of returning a reference demonstrated above is suitable. I was thinking along these lines:

1
2
3
4
5
6
 
At beginning of game, player objects are created, given initial values, and added to game class vector then during the game....

For each player in game::player_list
      current_player = get_current_player(player)    // Return reference to next player in list
      current_player.change_data(data)
next player


Is this a typical/good approach to this type of problem. Is there a down side that I am missing?

Thanks very much for your guidance,

Jimbot



Jimbot (29)

Also.....

dangrr888 I notice that in the Collection::AddElement function you pass in the element paramter by reference - is this to avoid the copying overhead of passing by value?

Thanks,

Jimbot
dangrr888 (80)
Hi Jimbot,

I finally got around to looking into your questions:

This was because I misunderstood the way classes handle data. I thought that because I had made all my element methods which can talk to private members of the element class, that when I added objects of this class to a vector in another array then they could still be accessed and changed as usual. I didn't appreciate that once I add them to a private member of another class then they also become private. (I hope that makes some sense)

Yes, everything within an object that is a private data member of a class will become private since the only way to access that member is through that class in which it is a member.

I wish to still keep this as I want to be able to edit element data during run time, and although I could add this functionality to the Collection class I think the code would lose some of its 'object orientated-ness'

What I realised is that if you don't want an Element to have its MyValue member changed then just declare it const. 

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
 
class Element
{
public:
  Element(int i = 5)
    : MyValue(i) {}
  
  int GetValue() const {return MyValue;}
  void SetValue(int NewValue) {MyValue = NewValue;}

private:
  int MyValue;
};

int main()
{
  const Element CElement(6);
  CElement.SetValue(8);

return 0;
}


error: passing ‘const Element’ as ‘this’ argument of ‘void Element::SetValue(int)’ discards qualifiers

However, if you append it to a non-const Collection and then try to alter it via an index operator, you can still
change its value of MyValue:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
 
#include <iostream>
#include <vector>
#include <cstddef>
#include <stdexcept>

using std::vector; using std::istream;
using std::cout; using std::endl;
using std::cin; using std::domain_error;

class Element
{
public:
  Element(int i = 5)
    : MyValue(i) {}
  
  int GetValue() const {return MyValue;}
  void SetValue(int NewValue) {MyValue = NewValue;}

private:
  int MyValue;
};

class Collection
{
public:
  Collection(vector<Element>::size_type i = 0)
    : AllElements(vector<Element>(i)) {}
  Collection(istream& is) {read(is);}

  istream& read(istream&);

  Element& operator[](vector<Element>::size_type i) 
  {
    if(i >= size())
      throw std::domain_error("index exceeds maximum value!");
    else
      return AllElements[i];
  }

  const Element& operator[](vector<Element>::size_type i) const
  {
    if(i >= size())
      throw std::domain_error("Element::operator[] - index exceeds maximum value!");
    else
      return AllElements[i];
  }

  void AddElement(const Element& NewElement = Element()) {AllElements.push_back(NewElement);}

  vector<Element>::size_type size() const {return AllElements.size();}

private:
  vector<Element> AllElements;
};

istream& Collection::read(istream& is)
{
  AllElements.clear();
  while(is)
    {
      int x;
      cout << "Please Enter an int: ";
      is >> x;
      if(is)
	AddElement(Element(x));
    }
  is.clear();

  return is;
}

int main()
{
  cout << "Creating a new (non-const) Collection object containing a const Element." << endl;
  const Element CElement(6);
  Collection NewCollection;
  NewCollection.AddElement(CElement);

  cout  << NewCollection[0].GetValue() << endl;
  NewCollection[0].SetValue(8);
  cout  << NewCollection[0].GetValue() << endl;

  return 0;
}



Creating a new (non-const) Collection object containing a const Element.
6
8


However, if you declare a const Collection you will not be able to change its contents beyond construction:

1
2
3
4
5
6
7
8
9
10
11
 
int main()
{
  cout << "Creating a new const Collection object containing 5 (default-initialised) Element objects." << endl;
  const Collection MyConstCollection(5);
  cout << "The value inside element 3 is: " << MyConstCollection[2].GetValue() << endl;
  cout << "Trying (and failing) to change the value inside element 3 to 7...." << endl;
  //MyConstCollection[2].SetValue(7); //if you try this the code won't compile because MyConstCollection is const
  cout << endl;

  return 0;
}



Creating a new const Collection object containing 5 (default-initialised) Element objects.
The value inside element 3 is: 5
Trying (and failing) to change the value inside element 3 to 7....



1
2
3
4
5
6
7
8
9
 
int main()
{
  ... 

  MyConstCollection[2].SetValue(7); //if you try this the code won't compile because MyConstCollection is const
  cout << endl;

  return 0;
}



error: passing ‘const Element’ as ‘this’ argument of ‘void Element::SetValue(int)’ discards qualifiers


As you can see from the code above, I have made some changes to the classes by including new constructors in Collection including a default that allows you to generate a Collection with a specified number of default-initialised Element objects (or none by default), and a constructor that allows you to read in integer values to initialise the Element objects from a specified input stream via the read function.

Also I have given Collection two index operators for const and non-const objects.

Here are some examples of the output using the above classes:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
 
int main()
{
  cout << "Creating a new (non-const) Collection object containing 5 (default-initialised) Element objects." << endl;
  Collection MyCollection(5);
  cout << "The value inside element 3 is: " << MyCollection[2].GetValue() << endl;
  cout << "Trying to change the value inside element 3 to 7...." << endl;
  MyCollection[2].SetValue(7);
  cout << "The value inside element 3 is now: " << MyCollection[2].GetValue() << endl;
  cout << endl;

  cout << "Creating a new (non-const) empty Collection, initialised from the standard input stream." << endl;
  Collection MyOtherCollection(cin);
  cout << endl;
  cout << "My new Collection contains " << MyOtherCollection.size() << " Elements." << endl;
  vector<Element>::size_type x = 3;
  if(x <= MyOtherCollection.size()) //to avoid throwing an exception due to access violation
    {
      cout << "The value inside element " << x << " is: " << MyOtherCollection[x-1].GetValue() << endl;
      cout << "Trying to change the value inside element " << x << " to 7...." << endl;
      MyOtherCollection[x-1].SetValue(7);
      cout << "The value inside element " << x << " is now: " << MyOtherCollection[x-1].GetValue() << endl;
    }
  cout << endl;

  return 0;
}



Creating a new (non-const) Collection object containing 5 (default-initialised) Element objects.
The value inside element 3 is: 5
Trying to change the value inside element 3 to 7....
The value inside element 3 is now: 7

Creating a new (non-const) empty Collection, initialised from the standard input stream.
Please Enter an int: 1
Please Enter an int: 2
Please Enter an int: 3
Please Enter an int: 6
Please Enter an int: 
My new Collection contains 4 Elements.
The value inside element 3 is: 3
Trying to change the value inside element 3 to 7....
The value inside element 3 is now: 7


Sorry for the long post, I hope that helps with your first set of questions.
dangrr888 (80)
Hi Jimbot,

regarding your question:

Is this a typical/good approach to this type of problem. Is there a down side that I am missing?


I would say that you should maybe store the Player objects in a vector in the Game class as you describe. Then access them and change them via index operators in the Game class.

Essentially, you could do this by changing the above Collection class to store Player objects instead of Element objects and make Player publicly inherit from Element. Or since Element is so simple, if you prefer, just expand upon it to create your Player class.

Hope this helps. Good luck!
dangrr888 (80)
dangrr888 I notice that in the Collection::AddElement function you pass in the element paramter by reference - is this to avoid the copying overhead of passing by value?


Yes, unless your dealing with objects of built-in type use pass-by-reference-to-const instead of pass-by-value if you don't wish to alter the object your passing.
Jimbot (29)

Hi dangrr888.

Thank you very much for your replies - I appreciate the time you have taken.

I am going to have a try with your examples and read through all the info, then will return with my thoughts :)

Cheers,

Jimbot
ne555 (10692)
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
 
class Collection{
//...
private:
  vector<Element> AllElements;
};

int main()
{
  cout << "Creating a new (non-const) Collection object containing a const Element." << endl; //that statement is false
  const Element CElement(6);
  Collection NewCollection; //as you can see in the definition of the class, it contains non-const Elements
  NewCollection.AddElement(CElement); //here you are copying the Element, it is not the same element

  cout  << NewCollection[0].GetValue() << endl;
  NewCollection[0].SetValue(8);
  cout  << NewCollection[0].GetValue() << endl;

  return 0;
}

1
2
3
4
5
6
7
 
  Element CurrentElement = MyCollection.GetElement(2); //copying the element
  cout << "The value inside element 3 is: " << CurrentElement.GetValue() << endl; //looking the copy

  cout << "Trying to change the value inside element 3 to 7...." << endl; //changing the copy
  CurrentElement.SetValue(7);

  cout << "The value inside element 3 is: " << CurrentElement.GetValue() << endl; //looking the copy

You could make an alias Element &CurrentElement = MyCollection.GetElement(2); and then they are just two names for the same object.

fun2code wrote:
Why make AllElements private if you want to be able to alter values stored in it from outside the class methods?
Well, it is a container class. A container class should provide a way to access the contained elements.
However you don't want to expose the implementation to the user (it could be dangerous, or you may want to change it)

Jimbot wrote:
Members of the player class need to be able to be edited as the game is played so I will need player member functions.
¿but who has the responsibility of edit them? Remember, getters and setters are evil http://www.javaworld.com/javaworld/jw-09-2003/jw-0905-toolbox.html
Last edited on
coder777 (8444)
ne555 wrote:
getters and setters are evil http://www.javaworld.com/javaworld/jw-09-2003/jw-0905-toolbox.html
That's such a pile of ...um... theorem

Let me see what I'm doing with the data
- serial port (sending/receiving)
- file (reading/writing)
- printer
- GUI
- time based collecting
- time based pushing
- tcp/ip (sending/receiving)
- bus (sending/receiving)
- ... pipe, ipc, GPRS ... etc

This all should be implemented in the data class. Now that'd be a massive interface (90% of the whole program).

The problem is that the structures of the data transmitted are very different. Am I supposed to implement that above for each an every data and transport channel available?

Yes that minimize the transport of the data, but maximize the transport of the programmer to the loony bin
ne555 (10692)
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
 
class input{
public:
  virtual void in(int&) = 0;  //basic data types
  virtual void in(long double&) = 0;
  virtual void in(char&) = 0;
  virtual void in(char*) = 0;
};

class output{
public:
  virtual void out(int) = 0;  //basic data types
  virtual void out(long double) = 0;
  virtual void out(char) = 0;
  virtual void out(const char*) = 0;
};

class ios: public input, public output{};

class printer: public output{};
class serial_port: public ios{};
class file : public ios{};
class gui: public ios{}; // well, actually you will have label, canvas, etc. to do i/o
class tcp_ip: public ios{};

class some_class{
public:
  void load( input &in ){
    in.in(name);
    in.in(value);
    //...
  }
  void show( output &out){
    out.out( resistance );
    out.out( power );
    //...
  }
};
The guy also suggest a facade, don't know what that is.

The idea is to know the responsibilities of each class.
Suppose that you need to simulate a battle. You could ask to each contender its stats (offensive power, agility, defensive, etc) but you will not say "hey, show me your equipment, and tell me your level so I can know calculate the damage you can inflict"

The problem is that the structures of the data transmitted are very different
¿But is the information the same?
Well a GUI may want to draw an sprite. I guess you could just ignore that parameter in the other representations, or you could make an extension
1
2
3
4
5
6
 
class some_class_with_gui: public some_class{
  public:
    void draw( gui& );
};
//or if you prefer
class some_class_with_gui: private some_class; //composition
Last edited on
coder777 (8444)
The guy also suggest a facade, don't know what that is.
According to this: http://en.wikipedia.org/wiki/Facade_pattern a facade is somewhat like a container.

The idea is to know the responsibilities of each class.
Suppose that you need to simulate a battle. You could ask to each contender its stats (offensive power, agility, defensive, etc) but you will not say "hey, show me your equipment, and tell me your level so I can know calculate the damage you can inflict"
My approach would be make it intuitive: Since the hero is the active one he asks the environment to give him a list of enemies. Then he calculates who's in reach and choses one and tells him the damage.

So the question is: who's active. And that's not the data. Active objects should indeed normally not have getters/setters

¿But is the information the same?
Different structure=different information. Everything else wouldn't make sense

1
2
3
4
 
class some_class_with_gui: public some_class{
  public:
    void draw( gui& );
};
This has a big flaw: When you're receiving data you must know about the nature of the further processing.
I decided to divide the data from it's processing. The data is passed by (smart) pointer so it's not too much of a slow down. namespaces provides the context.
ne555 (10692)
Then he calculates who's in reach and choses one and tells him the damage.
Yep, and then the enemy receives that damage, and calculate how much hp does it cost in base to his armor.
I can not think of a good bonus system, though.

¿what do you mean with active/passive objects?

The problem is that the structures of the data transmitted are very different. Am I supposed to implement that above for each an every data and transport channel available?
Yes, for every basic type. And you program that in the transmitter class. ¿do you know another way?

This has a big flaw: When you're receiving data you must know about the nature of the further processing.
Sorry, I can't understand you. ¿Could you rephrase it, please? (or provide an example)
Topic archived. No new replies allowed.