Question about vectors, iterators and vec.erase() and destructor

markyrocks (346)

Ok so I been playing around with vectors alot. I have been learning alot but have some specific questions.

First of which if I have a vector say of ints. Which is a class member. If I then take a value and push it into another vector of say pairs which is also a class member. Is it actually pushing in the value or is it pushing in a pointer to the already allocated memory?

I only ask bc I'm having issues with lost data. It maybe something else but just weird bc the problem didn't start acting this way until I destroyed the first vector...trying to get better habits of cleaning up 

1
2
3
4
5
6
 

vector<int> a;
vector<pair<<etc>> b;
a.push_back(1);
b.push_back(make_pair(a.back(),c))
a.~vector();


In a nut shell.

Also if I say have a vector of ints and just push back 0; to make a space can I then change the value in the future via vec.back()=1; or even dereferenceing an iterator and simply changing the dereferenced value?

If I have a vector that is a class member and use an iterator to erase an element do i have to say 

1
2
3
4
5
6
7
8
9
 
//member 
vector<int> a;
a.push_back(0)

auto it=this->a.begin();

this->a.erase(it);

this->a=&a.begin();


Thats probably not correct but from my understanding erasing an element reallocates a new block of memory if its not at the end it might not do it at the beginning but for the sake of argument pretend I'm erasing from the middle.
I'm losing the data somewhere and i ran out of time. Not home so can't play with it. I know if I make an iterator and erase from the middle the iterator becomes invalid. I basically have to reinitialize the iterator so I'm wondering if the same behavior is happening to my member variable? Just very odd getting into these.

Sry about dumb mistakes tried to do this quickly. I assure you they're not like that in the actual code


Edit also as a bonus question.

If I have a class member; if I change it inside of a class method what is the difference between referring to is just by its name itself or using this? Difference between this->var or just var? I realize if I have multiple instances of the class it could cause serious issues but what if i i just have one instance?

Should I get into the habit of something like if I have a class member int var. Initializing a temp pointer and doing one of these. int* temp=&this->var? I realize this is dependent on the variable type but I'm getting annoyed with data being lost bc im failing to ensure that its making it to the actual legit pointer location as opposed to ending up is some generic temp pointer location. Kinda driving me nutz
Last edited on
MikeyBoy (5631)
Is it actually pushing in the value or is it pushing in a pointer to the already allocated memory?

Where do you see any pointers anywhere in your code snippet?

Where have you ever heard that the compiler would ignore what the code says you're putting into a vector (which is obviously not a pointer) and would instead put a pointer into the vector, even though the type of object stored in the vector isn't a pointer?

You're fantasizing some really weird behaviour here, that would make no sense whatsoever.

In fact, as a quick glance at the documentation for std::vector would have told you, when you push an object into a vector, the vector stores a copy of the object, not a pointer or a reference to the original object.

Also:

a.~vector();

Why on earth are you explicitly calling the destructor on your object? That's not something you should ever do.

dutch (2548)
Don't write fake code for examples.
Only real code is useful.

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
 
#include <iostream>
#include <vector>
using namespace std;

int main() {

// Is this what you mean in your first example?
    vector<pair<vector<int>,int>> b;
    {
        int c = 1;
        vector<int> a{1,2,3};
        // Below, the pair stores a copy of a
        b.push_back(make_pair(a, c));
    } // 'a' dtor called here (don't call it explicitly!)
    
    // Obviously the copy of a is okay (if it's a proper copy)
    cout << b.front().first[1] << '\n'; // prints 2

// If you are storing a const reference to the vector instead.
    vector<pair<const vector<int>&,int>> bref;
    {
        int c = 1;
        vector<int> a{1,2,3};
        // Below, the pair stores a reference to a
        bref.push_back(make_pair(a, c));
    } // 'a' dtor called here

    // Now we have a problem since bref contains a reference to a,
    // which has been destoyed.
    // There's a good chance this statement will seem to work,
    // but it is undefined behaviour nonetheless.
    cout << b.front().first[1] << '\n'; // undefined behaviour


    vector<int> x {0, 1, 2, 3, 4};

    x.push_back(0);
    x.back() = 5;   // no problem (with vector of int, at least)

    // erasing an element moves all elements after it backwards,
    // "invalidating" any stored iterators to those elements.
    x.erase(x.begin());

    for (int n: x) cout << n << ' '; // prints 1 2 3 4 5
    cout << '\n';
}

Last edited on
markyrocks (346)
1
2
 
Where do you see any pointers anywhere in your code snippet?


I'm just spit balling bc I'm not entirely sure what exactly is happening in my code. I'm not home and noone really wants to look over 200 lines anyways

The documentation says..."Because vectors use an array as their underlying storage, erasing elements in positions other than the vector end causes the container to relocate all". I thought it said reallocates. Not relocate. Apparently what I missed was the return value on erase is the iterator to the next position. Thats kinda shitty tho bc in a for loop I'd basically skip that element next time around...

I will definitely admit that after messing with something for 12-15 hrs I do get delirious. Thanks dutch.
Last edited on
dutch (2548)
Thats kinda shitty tho bc in a for loop I'd basically skip that element next time around...

You just need to rearrange your for loop a little:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
 
#include <iostream>
#include <vector>
using namespace std;

int main() {
    vector<int> v{1, 2, 3, 4, 5, 6, 7};
    
    for (auto it = v.begin(); it != v.end(); )
        if (*it % 2 == 0)
            it = v.erase(it); // if erasing, use it's return value as the next it
        else
            ++it;             // otherwise increment it as usual

    for (auto n: v) cout << n << ' ';
    cout <<'\n';
}

Last edited on
markyrocks (346)
thats a very good point. I'm very tired today so my brain is not working at 100%. I just kinda gave up on erasing. I may get back to it but if i can avoid it i will. I was just experiencing one of those situations where a small logic error here small one there etc. add up to alot of stuff being badly broken and then you start trying to find the issues an changing stuff around ect. this leads to more small mistakes. blah. I finally had the first successful result.

Part of the issue was trying to build in too much stuff at one time i went back to just get a simple example to work and go from there. But hallelujah success. I'll put up the code in question just for lols. 

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
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
 

#include <Windows.h>
#include<iostream>
#include<vector>
using namespace std;

struct equation {
	
	vector<string> vecStrCombNum;
	vector<char> vecChars;
	vector<int> vecCombNum;
	vector<pair<float, char>> vecPairFlChar;
	int numCount;int operatorCount, parenthCount, parenthEndCount;
	
	equation() 
		: numCount(0),operatorCount(0), parenthEndCount(0),parenthCount(0), vecStrCombNum{}, vecChars{}, vecCombNum{}, vecPairFlChar{} {}

	void parseString(string equat) { 
		
		for (int i = 0; i < equat.length(); i++) {
			if (equat[i] != ' ') { this->vecChars.push_back(equat[i]); } //whitespace stripped 
		}
		size_t x = 0;
		for (auto i = this->vecChars.begin(); i < this->vecChars.end(); i++) {// this adds a * between any number thats up against a parenthesis
			if( *i == '(' && x > 0 && vecChars[x-1] != '+' && vecChars[x - 1] != '-' && vecChars[x - 1] != '*' && vecChars[x - 1] != '/' && vecChars[x - 1] != '%' && vecChars[x - 1] != '^'){
				i--; vecChars.insert(i, '*'); i++; this->operatorCount++;
			}
			if (*i == ')' && x + 1 < vecChars.size() && vecChars[x+1] != '+' && vecChars[x+1] != '-' && vecChars[x+1] != '*' && vecChars[x+1] != '/' && vecChars[x+1] != '%' && vecChars[x+1] != '^') {
				i++; vecChars.insert(i, '*'); i--;this->operatorCount++;
			}
			x++;
		}
		this->combineCharNumbers();
	}

	void combineCharNumbers() {  //looks though the vector or chars and groups char numbers in between operators together.
		string temp = "";
		//std::vector<char>::iterator it[20];
		int itCount = 0;
		for (auto i = this->vecChars.begin(); i < this->vecChars.end(); i++) {
			if (*i == '+' || *i == '-' || *i == '.' || *i == '*' || *i == '/'  || *i == '%' || *i == '^') {
				this->vecPairFlChar.push_back(make_pair(0.0, *i)); this->operatorCount++;
			}
			else if(*i == '('){ this->parenthCount++; this->vecPairFlChar.push_back(make_pair(0.0, *i));}
			else if (*i == ')') {this->parenthEndCount++; this->vecPairFlChar.push_back(make_pair(0.0, *i));}
			else if (*i != '+' && *i != '-' && *i != '.' && *i != '*' && *i != '/' && *i != '(' && *i != ')' && *i != '%' && *i != '^') {
				//while (*i != '+' && *i != '-' && *i != '.' && *i != '*' && *i != '/' && *i != '(' && *i != ')' && *i != '%' && *i != '^') { temp += *i; i++;}
				temp += *i; 
				 this->vecStrCombNum.push_back(temp); temp = "";
			}
		}
		
		this->combCharNumsToNums();
	}	

	void combCharNumsToNums() {
		int temp = 0;
		int temptotal;
		string* tempstr;
		for (int i = 0; i < this->vecStrCombNum.size(); i++) {

			for (int x = 0; x < this->vecStrCombNum[i].length(); x++) {
				tempstr = &this->vecStrCombNum[i];
				if ((*tempstr)[x] == '0') { temp = 0;  }
				else if ((*tempstr)[x] == '1') { temp = 1; (*tempstr)[x] = ' ';}
				else if ((*tempstr)[x] == '2') { temp = 2; (*tempstr)[x] = ' ';}
				else if ((*tempstr)[x] == '3') { temp = 3; (*tempstr)[x] = ' ';}
				else if ((*tempstr)[x] == '4') { temp = 4; (*tempstr)[x] = ' ';}
				else if ((*tempstr)[x] == '5') { temp = 5; (*tempstr)[x] = ' ';}
				else if ((*tempstr)[x] == '6') { temp = 6; (*tempstr)[x] = ' ';}
				else if ((*tempstr)[x] == '7') { temp = 7; (*tempstr)[x] = ' ';}
				else if ((*tempstr)[x] == '8') { temp = 8; (*tempstr)[x] = ' ';}
				else if ((*tempstr)[x] == '9') { temp = 9; (*tempstr)[x] = ' ';}
				
				if (x == 0) { temptotal = temp; }
				else if (x > 0) { temptotal = temptotal * 10 + temp; }
			}
			this->vecCombNum.push_back(temptotal); this->numCount++; temptotal = 0;
		}

		if (vecPairFlChar.size() < vecCombNum.size()) {
			while (vecPairFlChar.size() < vecCombNum.size()) { this->vecPairFlChar.push_back(make_pair(0.0, ' ')); }
		}
		for (auto i = vecPairFlChar.end(); i > vecPairFlChar.begin(); i--) {
			if (i == vecPairFlChar.end()) { i--; }
				if ( i._Ptr->second != ')' || i._Ptr->second != '(') {
					i._Ptr->first = this->vecCombNum.back(); this->vecCombNum.pop_back();
				}	
		}
		if (vecCombNum.size() > 0) { vecPairFlChar.front().first = vecCombNum.back(); vecCombNum.pop_back(); }
		if (operatorCount >= this->numCount) { cout << "error too many operators"; cin; return; }
		if (parenthCount != parenthEndCount) { cout << "error unbalanced parenthesis"; cin; return; }
		this->doMath();
	}

	void doMath() {
	
		int parenthStartPos;
		int parenthEndPos;

		//if (this->parenthCount > 0) {
		//	while (parenthCount != 0) {
	
		//		auto itbegin = this->vecPairFlChar.begin();
		//		auto itend = this->vecPairFlChar.begin();
		//			int tempCount = 0;
		//		for (int x = 0; x < this->vecPairFlChar.size(); x++) {  // this just finds the last ( and the first )
		//			itend++;
		//			if (tempCount != parenthCount) { itbegin++; }

		//			if (this->vecPairFlChar[x].second == '(') { parenthStartPos = x; tempCount++; }
		//			else if (tempCount == parenthCount && this->vecPairFlChar[x].second == ')') {
		//				parenthEndPos = x; x = this->vecPairFlChar.size();
		//			} //make it exit for loop
		//		}
		//		
		//		this->orderOfOperations(itbegin, itend);
		//		this->vecPairFlChar.erase(itbegin); this->vecPairFlChar.erase(itend); parenthCount--;
		//	}
		//}
		 this->orderOfOperations(); 
		cout << this->vecPairFlChar.back().first;//display
		this->vecPairFlChar.~vector();
	}

	void orderOfOperations() {


		auto x = this->vecPairFlChar.begin();
		size_t xx = 0;
		for (;;) {
			if (xx == vecPairFlChar.size() - 1) { break; }
			if (xx + 1 < vecPairFlChar.size() && this->vecPairFlChar[xx].second == '*' || this->vecPairFlChar[xx].second == '/') {
				this->vecPairFlChar[xx + 1].first = operations(this->vecPairFlChar[xx].first, this->vecPairFlChar[xx + 1].first, this->vecPairFlChar[xx].second);
				 x = this->vecPairFlChar.erase(x);
			}
			else { x++; xx++; }
		}
		
		for (size_t xx = 0; xx < vecPairFlChar.size(); xx++) {
			if (xx + 1 < this->vecPairFlChar.size() - 1 && this->vecPairFlChar[xx].second == '-' || this->vecPairFlChar[xx].second == '+') {
				this->vecPairFlChar[xx + 1].first = operations(this->vecPairFlChar[xx].first, this->vecPairFlChar[xx + 1].first, this->vecPairFlChar[xx].second);
			}
		}
	}

	float operations(float a, float b, char op) {
		if (op == '+') { return a + b; }
		else if (op == '-') { return a - b; }
		else if (op == '*') { return a * b; }
		else if (op == '/') { return a / b; }

		else if (op == '^') { return a + b; }
		else if (op == '%') { return a + b; }  //need work

	}

};

int main() {
	string str = "5+2*3+5/7+8";
	equation e;
	e.parseString(str);
}
Last edited on
dutch (2548)
I find the code somewhat hard to read.
Your variable names are pretty bad.
"temp" should be used rarely. Try to think of more meaningful names.
And encoding the types into the names is just clutter.
It has no meaning!
I know it's a vector of pairs of float and char, but what is it?
this-> is just clutter too.
Keep it simple.
You're over-complicating things.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
 
// this:
    tempstr = &this->vecStrCombNum[i];
    if ((*tempstr)[x] == '0') { temp = 0;  }
    else if ((*tempstr)[x] == '1') { temp = 1; (*tempstr)[x] = ' ';}
    else if ((*tempstr)[x] == '2') { temp = 2; (*tempstr)[x] = ' ';}
    else if ((*tempstr)[x] == '3') { temp = 3; (*tempstr)[x] = ' ';}
    else if ((*tempstr)[x] == '4') { temp = 4; (*tempstr)[x] = ' ';}
    else if ((*tempstr)[x] == '5') { temp = 5; (*tempstr)[x] = ' ';}
    else if ((*tempstr)[x] == '6') { temp = 6; (*tempstr)[x] = ' ';}
    else if ((*tempstr)[x] == '7') { temp = 7; (*tempstr)[x] = ' ';}
    else if ((*tempstr)[x] == '8') { temp = 8; (*tempstr)[x] = ' ';}
    else if ((*tempstr)[x] == '9') { temp = 9; (*tempstr)[x] = ' ';}
    
// can be this:
    int t = combNum[i][x] - '0'
    if (t > 0) combNum[i][x] = ' ';


Build it up a piece at a time.

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
 
#include <iostream>
#include <sstream>

using namespace std;

struct Token {
    char op;
    double val;
    bool is_op;

    Token(double val) : val(val), is_op(false) {}
    Token(char op)    : op(op),   is_op(true)  {}
};

bool next_token(istringstream& ss, Token& tok) {
    char ch;
    if (!(ss >> ch))
        return false;
    if (isdigit(ch)) {
        double val;
        ss.putback(ch);
        ss >> val;
        tok = Token(val);
    }
    else
        tok = Token(ch);
    return true;
}

ostream& operator<<(ostream& out, const Token& tok) {
    if (tok.is_op) cout << "op: "  << tok.op;
    else           cout << "num: " << tok.val;
    return out;
}

void tokenize(string s) {
    istringstream ss(s);
    for (Token tok{'\0'}; next_token(ss, tok); )
        cout << tok << '\n';
}


int main() {
    string eq {"(4 + 12) * 532 + 2"};
    string eq2 {"(4+12)*532+2"};
    tokenize(eq);
    tokenize(eq2);
}

MikeyBoy (5631)
At line 123, you're still trying to explicitly call a destructor:

this->vecPairFlChar.~vector();

Why are you doing this? As I've already said, this isn't something you should ever do. What is it you are trying to achieve here?
markyrocks (346)

MikeyBoy (5230)

At line 123, you're still trying to explicitly call a destructor:



Its then end of the program, figured it was safe to destroy it. I'll stop doing it if it serves no purpose.


I find the code somewhat hard to read.



Lol ya no doubt. I put FlChar in there so I knew which was in what position. Easy to try to do comparisons or whatever to the wrong position, first or second. I'm not saying this naming scheme is op or anything but it works for me.

The above needs reordered and stuff. The issues I was having prevented me from making it in the most logical way. Now that I have my issues with vectors sorted i can order everything to make the most sense. Im working on dealing with parentheses an it requires parts of the vector broken into a substring, evaluated and then returned as a number.
Last edited on
Ganado (6849)
Its then end of the program, figured it was safe to destroy it. I'll stop doing it if it serves no purpose.
The whole point of destructors is that they're automatically called when the lifetime of the object ends.
dhayden (5799)
These are exceptionally good questions that you're asking. A firm understanding of these issues is required to understand any language!
Is it actually pushing in the value or is it pushing in a pointer to the already allocated memory?
It's pushing the value. So the vector will contain a copy of the pushed value. Note that all the standard containers work this way. They all store values, not pointers. So std::SomeContainer<SomeType> always stores SomeType values, never pointers to SomeType.

You can create a container of pointers, but you do that explicitly: std::SomeContainer<SomeType *>. It's still storing values, but in this case, the value that its storing is itself a pointer.

can I then change the value in the future via vec.back()=1; or even dereferenceing an iterator and simply changing the dereferenced value?
Yes, and yes. vec.back() returns a reference to the last item in the vector. When you change the reference, you change the value in the vector. Just be sure you don't make this mistake:
1
2
 
int i = vec.back();  // copies the last item in vec to i, which is a different variable.
i = 5;  // assign 5 to i but leaves the last item in vec unchanged.


do i have to say

//member
vector<int> a;
a.push_back(0)
auto it=this->a.begin();
this->a.erase(it);
this->a=&a.begin();

You don't have to use this->. Also you don't have to do a = &a.begin(). The vector collection takes care of keeping itself consistent.

One thing you DO need to be aware of when inserting and deleting elements in a container is the effect upon any iterators that point to the container. The documentation for the insert() or erase() method will tell you which iterators may be come invalid. DO NOT rely on empirical evidence like "well I ran it twice and the iterator was still valid so I guess it always will be"). Sometimes the iterator only becomes truly invalid if the collection has to reallocate or otherwise move items around. In that case, your program may work sometimes and break other times. Read the documentation and act accordingly.

[When accessing a class member "var", what is the] difference between this->var or just var

There is no difference at all, provided that var is not also defined in an enclosing scope.

For example,
1
2
3
4
5
6
7
 
MyClass {
  int i;
  void f(int i) {
      i = 0;  // assigns to parameter i;
      this->i = 1;  // assigns to member variable i.
  }
};


Sometimes people use this-> for clarity. People like to do lots of prefixes and suffixes for clarity, but most are counterproductive in my opinion. Consider a reductio ad absurdum:
1
2
3
4
5
6
7
8
9
10
11
 
// Follow all Code Readability Guidelines!
class T_MyClass {                        // CRG 18.4.5: all types begin with "T_"
   ulli unsigned long long int ulli_m_x; // CRG Volume 8: variable names begin with type prefix (subsections 18.4-18.22: built in types)
                                         // CRG 3.978: member data should be prefixed by m_
                                         // CRG 4-43.E use "long int" and "short int" instead of "long" and "short"
   ulli unsigned long long int ulli_m_y; // CRG 8.334-Q: all variables must have their own type declaration.
   ulli unsigned long long int ulli_m_y; // Same as above
   unsigned long long int f() {
       return this->ulli_m_x + this ->ulli_m_y + this->ulli_m_z;  // CRG 1.45-D: always prefix members with this->
  }
};

which is intended to be more readable than:
1
2
3
4
5
6
7
 
class MyClass {
    unsigned long long x,y,z;
    unsigned long long f() {
        return x+y+z;
    }
  }
};


Should I get into the habit of something like if I have a class member int var. Initializing a temp pointer and doing one of these. int* temp=&this->var?
If you're doing it for performance then the answer is no. Let the compiler figure out the optimizations. It's better at it than you are. If you're doing it for readability, then use a reference instead, and make a note in the comments. Here's an actual example:
1
2
3
4
5
6
7
8
9
10
11
12
 
	Item &item(sj->items[i]);	// short hand
	if (item.attempts.empty()) {
	    continue;
	}
	rec.xqns[item.aci].push_back(dummyXqn);
	XqnInfo &xqn(rec.xqns[item.aci].back());  // short hand
	xqn.cc = item.cc;
	xqn.ac = item.ac;
	xqn.xqn = item.xqn;
	for (int j=0; j<item.attempts.size(); ++j) {
	    xqn.durations.push_back(item.attempts[j].duration);
	}

which is a lot more readable (to me anyway) than:
1
2
3
4
5
6
7
8
9
10
11
 
	if (sj->items[i].attempts.empty()) {
	    continue;
	}
	rec.xqns[sj->items[i].aci].push_back(dummyXqn);
	rec.xqns[sj->items[i].aci].back().cc = sj->items[i].cc;
	rec.xqns[sj->items[i].aci].back().ac = sj->items[i].ac;
	rec.xqns[sj->items[i].aci].back().xqn = sj->items[i].xqn;
	for (int j=0; j<sj->items[i].attempts.size(); ++j) {
	    rec.xqns[sj->items[i].aci].back().durations.push_back(sj->items[i].attempts[j].duration);
	}
    }


Happy coding!
markyrocks (346)
Thanks dhayden. The reason why I was considering using a temp pointer to a reference of this->var was bc i was having issues with lost data and i wasn't sure if i was like copying data to like a temporary pointer of this->var or something. My mind goes a little wild when stuff doesn't work the way its supposed to. Part of the reason for this is that I know that I don't know everything. That is fantastic that there's no difference between this->var and var bc i was honestly getting sick of putting this everywhere. Just clusters stuff up and my code is already bordering on unreadable.

@MikeyBoy. Ya i definitely need to use isdigit() and make my own isoperator() and isparentheses() function. That should clean things up quite a bit.
dhayden (5799)
i was having issues with lost data and i wasn't sure if i was like copying data to like a temporary pointer of this->var or something
I hear ya. It's really frustrating when data gets lost or overwritten. But if you understand how the std collections work, you can rule out some problems.
Topic archived. No new replies allowed.