My code is out of bounds but I don't know how to fix it.

mpelle (8)
This is my first time using a forum like this, but I'm desperate. My code below is meant to delete either the odds or evens in a vector of randomly generated numbers. It is out of bounds in my deleteFromVector function and I don't know how to fix it. I am only a beginner and I don't know if I am doing this right. I'm not great at coding and I'm not familiar with this site. If anyone has helpful tips or advice, I will be beyond grateful. I am only allowed to use member functions pop_back, resize, size, and push_back, which makes my code very large and complicated.

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
 
//Madyson Pellegrino 05/11/2020
#include <iostream>
#include <vector>
#include <ctime>
#include <cstdlib>
using namespace std;

const int MAX = 50;
const int MIN = 0;
const int SIZEMAXIMUM = 25;
const int ODDDELETER = 1;
const int EVENDELETER = 2;
//--------------------------------------------------------------------------
void deleteFromVector (vector <int> &vector, int decision)
{
    if (decision == ODDDELETER)
    {
      for(int indexOriginal = 0; indexOriginal < vector.size(); indexOriginal++)
        {
            if (vector[indexOriginal] % 2 != 0)
            {
                for (int indexCopy = indexOriginal; indexCopy < vector.size(); indexCopy++)
                {
                    vector[indexCopy] = vector[indexCopy + 1];
                }
                
                vector.resize(vector.size() - 1);
                indexOriginal--;
            }
        }  
    }
    
    if (decision == EVENDELETER)
    {
      for(int indexOriginal = 0; indexOriginal < vector.size(); indexOriginal++)
        {
            if (vector[indexOriginal] % 2 == 0)
            {
                for(int indexCopy = indexOriginal; indexCopy < vector.size(); indexCopy++)
                {
                    vector[indexCopy] = vector[indexCopy + 1];
                }
                
                vector.resize(vector.size() - 1);
                indexOriginal--;
            }
        }  
    }
}
//--------------------------------------------------------------------------
void displayVector (const vector <int> &vector)
{
    for(int index = 0; index < vector.size(); index++)
    {
        cout<< vector[index]<< ' ';
    }
}
//--------------------------------------------------------------------------
void loadVector (vector <int> &vector)
{
    int seed = time (0);
    srand(seed);
    
    int num = rand () % (MAX - MIN + 1) + MIN;
    
    for (int index = 0; index < SIZEMAXIMUM; index++)
    {
        vector.resize(vector.size() + 1);
        vector[vector.size() - 1] = num;
        
        num = rand () % (MAX - MIN + 1) + MIN;
    }
      
}
//--------------------------------------------------------------------------
int main()
{
    vector <int> vector (0);
    int decision;
    
    loadVector(vector);
    
    displayVector(vector);
    
    cout<< endl<< endl<< "The size of the vector is: "<< vector.size();
    
    cout<< endl<< endl<< "Enter 1 to delete odds. Enter 2 to delete evens: ";
    cin>> decision;
    
    cout<<endl;
    
    deleteFromVector(vector, decision);
    
    displayVector(vector);
    
    cout<< endl<< endl<< "The size of the vector is: "<< vector.size();
    
    
    return (0);
}


/*

TEST CASES:

21 20 5 7 37 16 0 37 19 14 21 26 15 5 7 46 39 29 30 0 14 27 4 36 45 

The size of the vector is: 25

Enter 1 to delete odds. Enter 2 to delete evens: 1

20 16 0 14 26 46 30 0 14 4 36 

The size of the vector is: 11

=================================================================================

50 14 1 2 18 15 46 16 18 40 17 33 23 40 46 16 34 11 29 14 2 27 28 46 3 

The size of the vector is: 25

Enter 1 to delete odds. Enter 2 to delete evens: 1

50 14 2 18 46 16 18 40 40 46 16 34 14 2 28 46 

The size of the vector is: 16

=================================================================================

28 0 40 27 8 18 34 40 27 41 43 0 10 1 30 41 18 11 14 14 7 18 14 43 27 

The size of the vector is: 25

Enter 1 to delete odds. Enter 2 to delete evens: 2

27 27 41 43 1 41 11 7 43 27 

The size of the vector is: 10

=================================================================================

30 10 9 31 25 12 7 50 35 26 13 45 22 26 29 3 3 14 48 12 37 18 33 45 3 

The size of the vector is: 25

Enter 1 to delete odds. Enter 2 to delete evens: 2

9 31 25 7 35 13 45 29 3 3 37 33 45 3 

The size of the vector is: 14

*/
Last edited on
Ganado (6849)
Please edit your post and add formatting
[code]
    {your code here}
[/code]
Last edited on
mpelle (8)
For some reason the indenting got all messed up, my apologies.
Ganado (6849)
That's why you should edit your original post and add formatting,
[code]
    {your code here}
[/code]


This appears to be the first place you go out of bounds:
1
2
3
4
 
for (int indexCopy = indexOriginal; indexCopy < vector.size(); indexCopy++)
{
    vector[indexCopy] = vector[indexCopy + 1];
}

Notice is that indexCopy can be at most equal to vector.size()-1, but vector[indexCopy + 1]; will then be vector[vector.size()]; which is out of bounds.

Later on, you do the same thing again.

Also,
1
2
 
vector.resize(vector.size() + 1);
vector[vector.size() - 1] = num;
Small tip: This is doing what push_back already does (just less efficiently).
Last edited on
deleted account xyzzy (5768)
The indenting became "messed up" because you didn't use code tags.

http://www.cplusplus.com/articles/jEywvCM9/

HINT: you can edit your post and add code tags.

There are also output tags, you should use those for your "test cases."

http://www.cplusplus.com/articles/z13hAqkS/
mpelle (8)
Thank you for your guys' help regarding properly formatting my question!!!
mpelle (8)
I'm not sure how to reply to people, so again sorry if I'm being obnoxious, but yes, I know that is where I go out of bounds, and I've been trying to manipulate that section of code (I also don't really know how the correct terminology for coding and such) so that it doesn't go out of bounds. It either messes up the entirety of my code or simply doesn't work. Does this make it not go out of bounds?

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
 
if (decision == ODDDELETER)
    {
      for(int indexOriginal = 0; indexOriginal < vector.size(); indexOriginal++)
        {
            if (vector[indexOriginal] % 2 != 0)
            {
                for (int indexCopy = indexOriginal; indexCopy < vector.size() - 1; indexCopy++)
                {
                    vector[indexCopy] = vector[indexCopy + 1];
                }
                
                vector.resize(vector.size() - 1);
                indexOriginal--;
            }
        }  
    }
Last edited on
Handy Andy (5051)
Hello mpelle,

You should check into http://www.cplusplus.com/reference/vector/vector/erase/ You are going to more work than you need to.

In the line vector[indexCopy] = vector[indexCopy + 1]; "indexCopy + 1" will put you past the end of the vector.

While I am think about it choose another name than "vector". It does get a bit confusing. Something like "nums" or "vNums" where the "v" means vector.

I tried this for the odd numbers:
1
2
3
4
5
6
7
 
for (auto it = vector.begin(); it != vector.end(); )
{
	if (*it % 2 != 0)
		it = vector.erase(it);
        else
            it++;
}

The erase function will erase, or delete, the given element and the vector will automatically resize its-self. No need for all that code you have which does not work.

Andy

lastchance (6980)
https://en.wikipedia.org/wiki/Erase%E2%80%93remove_idiom

Use the remove_if version in this instance.
Last edited on
mpelle (8)
I am not allowed to use member functions I have not learned yet, so I can only use pop_back, resize, size, and push_back.
Handy Andy (5051)
Hello mpelle,

I am not allowed to use member functions I have not learned yet
. Well that is stupid. I may have a different way of looking at things, but you were told that you needed the header file"vector" for this program. Once you include this file you have access to all that the header file has to offer even if it is not taught in class. Some of that you will have to learn on your own. If you wait for something to be taught in class it may take years and still may not be covered.

As I have had the time to work with your program I have found this:
1
2
3
4
 
int main()
{
	vector <int> vector(0);
	int decision;


I changed it to this:
1
2
3
4
5
6
 
int main()
{
	std::vector<int> vNums;
	int decision;

	srand(static_cast<size_t>(time(nullptr)));  // <--- Only needs done once. And not in a function.

I originally put the "std::" to distinguish between the class being used as the type and the variable name, but since I changed the variable name you can remove the "std::" if you want. The space beterrn "vector" and "<int>" is not needed, but makes no difference to the compiler. More often I see things like this written without the space.

After watching this video https://channel9.msdn.com/Events/GoingNative/2013/rand-Considered-Harmful I found this is the best way to write "srand". When you look up "srand" you will find that its parameter is an "unsigned int", size_t is an alias for "unsigned int", and "time" does not return an "int" or "unsigned int" hence the type cast. Easy to miss, but the video does mention that "nullptr" is the better parameter form C++11 on. There is more than 1 way to write "srand", but these are likely to produce warnings at compile time and should be addressed.

The rest of "main" works and I have not looked closely at what is there to see if it is worth changing.

The first function called is loadVector(vNums);. Looking at that function you can use what you have, but as Ganado said

Also,
1
2
 
vector.resize(vector.size() + 1);
vector[vector.size() - 1] = num;

Small tip: This is doing what push_back already does (just less efficiently).


The function can be written as:
1
2
3
4
5
6
7
8
9
 
void loadVector(std::vector<int> &vNums)
{
	//int seed = time(0);  // <--- "time(0)" does not return an "int" or the "unsigned int" that "srand" needs.
	//srand(seed);  // <--- Should be in "main"

	for (int index = 0; index < SIZEMAXIMUM; index++)
		vNums.push_back(rand() % (MAX - MIN + 1));

}

The way this works is that "rand" will generate a number before it is put at the end of the vector with "push_back".

Another way to write this would be:
1
2
3
4
5
6
7
8
9
10
11
12
 
void loadVector(std::vector<int> &vNums)
{
	//int seed = time(0);  // <--- "time(0)" does not return an "int" or the "unsigned int" that "srand" needs.
	//srand(seed);  // <--- Should be in "main"

	for (int index = 0; index < SIZEMAXIMUM; index++)
        {
                int num = rand() % (MAX - MIN + 1);

		vNums.push_back(num);
        }
}

The 2 lines commented out ar there to show you that they do not belong in the function.

In either bit of code the "push_back" function will add to the end of the vector and the vector class will keep track of the new size of the vector.

In the other functions where you use for (int index = 0; index < vector.size(); index++). The ".size()" function will return a "size_type" variable. This is defined as:

Member type size_type, or size_t, is an unsigned integral type.

Usually not a problem. It should produce a warning, but will not stop the program from running.

Also usually best to keep the types the same. This would be a better way to write these lines:
for (size_t index = 0; index < vector.size(); index++)

If there is anything you do not understand just ask.

In the "deleteFromVector" function the 2 if statements could be changed to an if/else/else. I ended up with this:
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
 
bool deleteFromVector(vector<int> &vNums, int decision)
{
	if (decision == ODDNUMS)
	{
		for (auto it = vNums.begin(); it != vNums.end();)
		{
			if (*it % 2)
				it = vNums.erase(it);
			else
				it++;
		}
	}
	else if (decision == EVEN_NUMS)
	{
		for (auto it = vNums.begin(); it != vNums.end();)
		{
			if (!(*it % 2)/**it % 2 != 0*/)
				it = vNums.erase(it);
			else
				it++;
		}
	}
	else
	{
		std::cout << "     Invalid Choice!\n";

		return true;
	}

	return false;
}


Andy
mpelle (8)
You really do not understand how much I appreciate the effort you have put into helping me with my problem, and I feel really bad that I have to disregard the majority of it. My teacher does not allow us to use the internet to help us, and I must make sure she doesn't find out so I can't use any member functions that we haven't discussed in class yet or she'll know. I don't understand a lot of what you said as we have not covered those topics yet and they are covered in the AP Computer Science course in my school, I'm only in the intro course. My teacher is very strict and I'll get a 0 if I use stuff we have yet to learn. I also have to maintain my "coding style", cause she picks up very easily on things that seem off about our code and usually finds out if someone uses the internet or gets help from another student, thus why I used the 

1
2
 
vector.resize(vector.size() + 1);
vector[vector.size() - 1] = num;


instead of push_back, because I'm nervous she'll call me out for cheating.

Again, thank you so much for your help and I'm sorry I've wasted your time. You went through so much effort to help me and I can't do much with it. I hope you have a good rest of your day and you and your loved ones stay safe.
Ganado (6849)
Here's another way of doing it.
So you're trying to delete, say, all the odd numbers from the vector. Let's just focus on that.
input vector: 1 3 7 4 6 
output vector: 4 6


In your previous code, you have a for loop nested in another for loop. As far as I can tell, this appears to be unnecessary.

What I would do is, loop through the vector with a simple, single loop, and have a variable that keeps track of what the next free slot available in the array is.
Afterwards, resize the vector to remove the later slots that are no longer used.

Here's what I mean in 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
 
// Example program
#include <iostream>
#include <vector>
using namespace std;

void delete_odd_numbers(vector<int>& vec)
{
    size_t output_index = 0;
    for (size_t i = 0; i < vec.size(); i++)
    {
        if (vec[i] % 2 == 0) // even, KEEP
        {
            // keeping it, put it in first free spot
            vec[output_index] = vec[i];
            output_index++;
        }
        
        // else: It's odd, simply don't increment output_index
        // this odd element will be overwritten by either an element further down
        // or when the vector is resized at the end
    }
    // Note: output_index is now the number of elements in the EVEN vector
    vec.resize(output_index);
}

// Helper function just for demonstration
void print(const vector<int>& vec)
{
    for (size_t i = 0; i < vec.size(); i++)
    {
        cout << vec[i] << ' ';   
    }
    cout << '\n';
}

int main()
{
    vector<int> vec = { 1, 3, 7, 4, 6 };
    print(vec);
    delete_odd_numbers(vec);
    print(vec);
}

1 3 7 4 6 
4 6 
1 3 8 7 4 6 
8 4 6


The actual logic is only 6 lines. Hopefully that gives you something to experiment with.
Last edited on
Topic archived. No new replies allowed.