Vector Not Printing

[syeare]

What I must do:

Make a program that makes a double vector with at least 30 negative and positive elements. I also must have functions to calculate/print the standard deviation (that one I finished earlier here, thank you dutch: http://www.cplusplus.com/forum/beginner/273688/ ), the mean value, the highest value, and the lowest value of the vector. Next, I will need a split() function to split the values of the double vector into 2 new double vectors, positive and negative (the former will have all values from 0 - max_positive_value while the latter will have all nonzero negative values). Finally, I need one last function to display both of these modified vectors.

My Issue:

Nothing is being printed from my split() function! For debugging purposes, I made it so that it prints all the values of the positive and negative vectors, but it isn't printing anything!

Code Block in Question:

double split(const vector<double> &data)
{
    vector <double> positive;
    vector <double> negative;
    int j = 0; // iterate inner

    // add to pos/neg vector depending on value of data[i]
    for (int i = 0; i < data.size(); ++i)
    {

        if (data[i] >= 0)
        {
            positive[j] = data[i];
            j++;

        } else if (data[i] < 0)
        {
            negative[j] = data[i];
            j++;
        }
    }

    // debugging, print vectors
    for (int vC = 0; vC < positive.size(); ++vC)
    {
        cout << positive[vC] << endl;
    }

    for (int vC = 0; vC < negative.size(); ++vC)
    {
        cout << positive[vC] << endl;
    }

    return 0;

}

Full code:

#include <iostream>
#include <vector>
#include <ctime> // both this and the next header are needed for
#include <cstdlib> // rand() to randomly generate numbers 
#include <cmath> // sqrt

using namespace std;

double split(const vector<double> &data)
{
    vector <double> positive;
    vector <double> negative;
    int j = 0; // iterate inner

    // add to pos/neg vector depending on value of data[i]
    for (int i = 0; i < data.size(); ++i)
    {

        if (data[i] >= 0)
        {
            positive[j] = data[i];
            j++;

        } else if (data[i] < 0)
        {
            negative[j] = data[i];
            j++;
        }
    }

    // debugging
    for (int vC = 0; vC < positive.size(); ++vC)
    {
        cout << positive[vC] << endl;
    }

    for (int vC = 0; vC < negative.size(); ++vC)
    {
        cout << positive[vC] << endl;
    }

    return 0;

}

// calculate std deviation
double calculateSD(const vector<double> &data)
{
    // calculate sum of data
    double sum = 0.0;
    for (size_t i = 0; i < data.size(); ++i)
        sum += data[i]; 

    // calculate average
    double avg = sum / data.size();
 
    // calculate and return the standard deviation
    double stdDev = 0.0;
    for (size_t i = 0; i < data.size(); ++i)
        stdDev += (data[i] - avg) * (data[i] - avg);
    return sqrt(stdDev / data.size());
}

int main()
{
    // vector with minimum 30 values
    vector <double> data (30);

    // seed rand() with time
    srand (time(NULL));
    
    // fill vector with neg/pos num
    for (int i = 0; i < data.size(); ++i)
    {
        data[i] = rand() - (RAND_MAX / 2);
        // cout << data[i] << endl;
    }

    split(data);

}

(EDIT: Fixed some formatting; also, I have not yet made the functions for finding the mean/highest/lowest value of the data vector )

[Handy Andy]

Hello syeare,

What I see in the "split" function is:

In the first bit of code lines 3 and 4 define a vector, but it has no size, i.e., no elements.

In lines 13 and 18 you are trying to access a if it has a fixed number of elements like an array does. What you need in place of positive[j] = data[i]; is positive.push_back(data[i]) which will add to the vector until "data" has reached the end.

A possible problem is that lines 3 and 4 are local variables and will be destroyed when the function, so if you need these vectors outside the function consider defining them in "main" and passing them to the function like you did "data", but without the "const" part.

In lines 24 and 29 change the "int" to "size_t" as the ".size()" function returns a "size_t" type variable. This way in the middle conparison the variable types will match based on your header files and how "size_t" is defined. You will also need to change everywhere you use a ".size()" function

Andy

[syeare]

Hey Andy,

Thanks for the help! I didn't know about the size_t thing before since I don't really have much experience with vectors. The great thing is that it DOES print now, but some reason doesn't store the negative numbers of the data vector in the negative array. It outputs something like this:

 -9828 14911 4496 -3998 -2992 -9400 4678 7435 -11469 15429 -3682 1621 -10601 -2929 154 13327 
13984 11532 4254 3547 -5895 16139 9926 4403 3781 14959 -1669 10409 -295 5904 rand end14911 4496 
4678 7435 15429 1621 154 13327 13984 11532 4254 3547 16139 9926 4403 3781 14959 10409 5904 14911 
4496 4678 7435 15429 1621 154 13327 13984 11532 4254

I made the changes that you suggested. As for lines 3 and 4, I was intending to figure out some way to return these vectors since I have to use as the parameters for some other functions (namely, to find the largest/lowest and the mean). The new source:

#include <iostream>
#include <vector>
#include <ctime> // both this and the next header are needed for
#include <cstdlib> // rand() to randomly generate numbers 
#include <cmath> // sqrt

using namespace std;

double split(const vector<double> &data)
{
    vector <double> positive;
    vector <double> negative;
    int j = 0; // iterate inner

    // add to pos/neg vector depending on value of data[i]
    for (size_t i = 0; i < data.size(); ++i)
    {

        if (data[i] >= 0)
        {
            positive.push_back(data[i]);
            j++;

        } else if (data[i] < 0)
        {
            negative.push_back(data[i]);
            j++;
        }
    }

    // debugging
    for (size_t vC = 0; vC < positive.size(); ++vC)
    {
        cout << positive[vC] << endl;
    }

    for (size_t vC = 0; vC < negative.size(); ++vC)
    {
        cout << positive[vC] << endl;
    }

    return 0;

}

// calculate std deviation
double calculateSD(const vector<double> &data)
{
    // calculate sum of data
    double sum = 0.0;
    for (size_t i = 0; i < data.size(); ++i)
        sum += data[i]; 

    // calculate average
    double avg = sum / data.size();
 
    // calculate and return the standard deviation
    double stdDev = 0.0;
    for (size_t i = 0; i < data.size(); ++i)
        stdDev += (data[i] - avg) * (data[i] - avg);
    return sqrt(stdDev / data.size());
}



int main()
{
    // vector with minimum 30 values
    vector <double> data (30);

    // seed rand() with time
    srand (time(NULL));
    
    // fill vector with neg/pos num
    for (size_t i = 0; i < data.size(); ++i)
    {
        data[i] = rand() - (RAND_MAX / 2);
        cout << data[i] << endl;
    }
    cout << "rand end";

    split(data);

}

EDIT: Not sure why the output veers off the screen like that, but oh well

[syeare]

In my main() function on L77, should I also change that to data.push_back(rand() - (RAND_MAX / 2)); ? I am not completely sure, but it doesn't SEEM like it causes an issue. Is it because I already declared the size of the vector and it's not empty, so that's why I can access any of its empty elements?

[Handy Andy]

Hello syeare,

While working on the program I noticed this:

    // debugging
    for (int vC = 0; vC < positive.size(); ++vC)
    {
        cout << positive[vC] << endl;
    }

    for (int vC = 0; vC < negative.size(); ++vC)
    {
        cout << positive[vC] << endl;
    }

See any problem with line 9?

Also I like to use this with "srand": srand(static_cast<unsigned int>(time(nullptr)));. "stand" takes an "unsigned int" as its parameter, but "time()" does not return an "unsigned int". In more modern C++ the "nullptr" is the better choice even though what you have still works.

For more information on using "rand" and its problems
https://channel9.msdn.com/Events/GoingNative/2013/rand-Considered-Harmful
and
https://web.archive.org/web/20180123103235/http://cpp.indi.frih.net/blog/2014/12/the-bell-has-tolled-for-rand/


Andy

[syeare]

POSITIVE! WHAT A STUPID MISTAKE OF MINE

[Handy Andy]

Hello syeare,

In "main" I have done this:

int main()
{
    constexpr int MAXSIZE{ 30 };

    // vector with minimum 30 values
    vector <double> data;  // <--- Do not give this a starting size.
    vector <double> positive;
    vector <double> negative;

    // seed rand() with time
    //srand(time(NULL));
    srand(static_cast<unsigned int>(time(nullptr)));

    // fill vector with neg/pos num
    for (int i = 0; i < MAXSIZE; ++i)
    {
        data.push_back(rand() - (RAND_MAX / 2));
        // cout << data[i] << endl;
    }

    split(data, positive, negative);
}

In the "split" function I started with this:

// <--- Should be a void function. No reason to return anything.
void split(const vector<double> &data, vector<double> &positive, vector<double> &negative)
{
    //int j = 0; // iterate inner <--- Does this hae a use?

    // add to pos/neg vector depending on value of data[i]
    for (size_t i = 0; i < data.size(); ++i)
    {
        if (data[i] >= 0)
        {
            positive.push_back(data[i]);
            //j++;
        }
        else if (data[i] < 0)
        {
            negative.push_back(data[i]);
            //j++;
        }
    }
// ... more code follows. 

By passing the last 2 vectors by reference you do not have to return anything. Any how the problem is that a function can only return 1 thing. That is the best way of looking at it for now because any code that could return both vectors is likely beyond where you are at right now.

And for fun, since I am figuring that you will not keep this part I did:

// debugging
std::cout << "\n Positive numbers\n";

for (size_t vC = 0; vC < positive.size(); ++vC)
{
    cout << std::setw(8) << positive[vC] << endl;
}

Printing the other vector is similar, but needs fixed.

This produce an output of:

 Positive numbers
   14697
   11962
    2808
   11916
    8027
   11899
     145
    9614
     447
   11222
    4554
   14137
    1672
    7255
   15032

 Negative numbers
   -2937
  -14175
  -14304
  -14419
   -8731
   -4397
  -13634
   -3745
   -5157
   -6862
   -2918
   -3659
   -5933
  -14597
   -5992

Of course this changes on any given run.

To your other question. When you define vector <double> data (30);. You are creating a vector with 30 elements all set to 0.0. Later when you do the "push_back" you are just adding to the original 30 and end up with a vector that has a size if 60. No what you want.

Andy

[Handy Andy]

Hello syeare,

POSITIVE! WHAT A STUPID MISTAKE OF MINE

No worries. It happens to the best of us now and then.

Andy

Edit:

The way I noticed is the when "positive.size()" was less than "negative.size()" the second for loop was trying to access the "positive" vector beyond the end of its size.

[syeare]

Sorry, but I am not clear on why you did this:

constexpr int MAXSIZE{ 30 };

Additionally, on L21 of that same codeblock, why do you pass them in as direct parameters? I read that passing by reference uses significantly less resources (I'm sure modern computers don't have much issue dealing with a few extra bytes here and there, but I previously thought that the point still stood) than passing in a parameter directly. I thank you for your time thus far and going forwards!

( If you are curious as to what my final product looks like, it is saved at this location: https://pastebin.com/JRPKttEg )

[seeplus]

In split(), the second if isn't needed as if the number is not >= then it must be <. Also, a range-based for is easier

void split(const vector<double>& data, vector<double>& positive, vector<double>& negative)
{
	for (double d : data)
		if (d >= 0)
			positive.push_back(d);
		else
			negative.push_back(d);
}

[Handy Andy]

Hello syeare,

constexpt started with the C++ 2011 standards. It is similar to using const. VAriables defined as constant are usually in all capital letters, but not a requirement.

The point of defining at the beginning of the function, or near the beginning of the file, is that you only have 1 place to change this variable and everywhere it is used in the code will see this change.

An example would be let's say that you have 1000 to 1500 lines of code and that you have many for loops like: for (int i = 0; i < 10; i++). Now you need to go through these 1000+ lines of code to find all the for loops to change the 10 to 20, but you will likely miss 1 or 2 that you will find when the program runs and some output is short.

Using for (int i = 0; i < MAXSIZE; i++) in all the for loops you only have 1 place to change the size of the variable and it will been seen everywhere "MAXSIZE" is used and use the new value of "MAXSIZE".

It is a simple thing that makes writing the code easier.

I think you are referring to split(data, positive, negative);. This is the function call and you only have to send the variable name(s) for this. At the function definition:
void split(const vector<double> &data, vector<double> &positive, vector<double> &negative). Here it is saying that the variables are being received by reference and that you will be using the variables where they were first defined, in this case "main". The opposit would be to leave off the "&" which means that the variables are being received by value, or it is a copy of the variable passed to the function, which can get expensive to create the extra memory needed just to use it in the function.

When it comes to "std::string"s, "std::vector"s and other container classes it is better to pass by reference and not have to make an expensive copy.

When I looked at your link I noticed that in the "split" function several things.

First you do make use of blank lines, but in several places the double spacing is not needed. You kind of over did it there.

You define the "int j", but it does not appear to have any use.

A function should do 1 thing, do it well and return. The last lines of the function calling the other functions should not be here. They should be in "main" where "main" is designed to set up and direct the program flow to the other functions. Which is why you would define the vectors "positive" and "negative" in "main" and then pass them to the functions that need them.

At first glance I did not see anything that might be a big problem, but I will copy the code and give it a test and let you know what I find.

Andy