Redempter
Redempter

Reputation: 43

C++: Bug when trying to fill dynamic vector with double

I'm having trouble with a program I'm doing in class and even the teacher can't find the problem. We're doing a program that ask the user to enter double then when he stop, it scan the array and separate the positive and negative to put them in different arrays.

We notice that when we use float the program work for more numbers but still bug if we enter too much and if we use double it bug after only a few numbers. By bug I mean, the program do well but when it display the result there is some weird numbers in the array. Here is the code using double:

#include <iostream>
using namespace std;

void filling(double *, int &);
void sortPositiveNegative(double *, double *, double *, int, int &, int &);
void display(const double *, int);

int main () {
    double * vecteur = new double;
    double * positive = new double;
    double * negative = new double;
    int counter = 0, counterPos = 0, counterNeg = 0;

    cout << "Filling of the real number vector " << endl;
    filling(vecteur, counter);

    cout << endl << "Display of the real number vector " << endl;
    display(vecteur, counter);

    cout << endl << "Sort of the positive and negative in the real number vector: " << endl;
    sortPositiveNegative(vecteur, positive, negative, counter, counterPos, counterNeg);

    cout << endl << "Display of the positive real number : " << endl;
    display(positive, counterPos);

    cout << endl << "Display of the negative real number : " << endl;
    display(negative, counterNeg);

    system("PAUSE");
    return 0;
}

void filling (double *vecteur, int &counter)
{
    bool validation;
    char choice = 'Y';
    do
    {
        do
        {
            validation = true;
            cout << "Please enter the value of case " << counter+1 << ": ";
            cin >> vecteur[counter];
            if(cin.fail())
            {
                cerr << "The number entered is not valid." << endl;
                cin.clear();
                validation = false;
            }
            while(cin.get() != '\n'){}
        }while(!validation);
        counter++;
        do
        {
            validation = true;
            cout <<"Do you wish to continue? (Y/N): ";
            cin >> choice;
            if(toupper(choice) != 'Y' && toupper(choice) != 'N')
            {
                cerr << "We don't understand your choice, please try again." << endl;
                cin.clear();
                validation = false;
            }
            while(cin.get() != '\n'){}
        }while(!validation);
    }
    while(toupper(choice)=='Y');
}

void sortPositiveNegative(double *vecteur, double *positive, double *negative, int counter, int &counterPos, int &counterNeg)
{
    int i = 0;
    for(i; i<counter;i++)
    {
        if(vecteur[i] >= 0)
            positive[counterPos++] = vecteur[i];
        else
            negative[counterNeg++] = vecteur[i];
    }
}

void display (const double *vecteur, int counter)
{
    for(int i = 0; i<counter;i++)
        cout << vecteur[i]<<endl;
    cout << endl;
}

My teacher think it might be a memory problem but we have no idea why it's doing that.

Thanks in advance.

Upvotes: 3

Views: 505

Answers (3)

syam
syam

Reputation: 15089

double * vecteur = new double;
double * positive = new double;
double * negative = new double;

Here you allocate exactly ONE double each time. The first item you store in your "array" is fine, but anything else after that is undefined behaviour.

The fix is to actually allocate as many items as you need:

double * vecteur = new double[MAXIMUM8NUMBER_OF_ITEMS];
double * positive = new double[MAXIMUM8NUMBER_OF_ITEMS];
double * negative = new double[MAXIMUM8NUMBER_OF_ITEMS];

Or better yet, use a standard container like std::vector.

Upvotes: 2

alexrider
alexrider

Reputation: 4463

double * vecteur = new double;

You allocate space for one doubile

filling(vecteur, counter);

Pass it to filling

    cin >> vecteur[counter];

And fill untill user presses Y going outside of array with one element you have allocated memory for.
double vs float doesn't matter much. float is simply smaller and thus will be corrupting memory slower. But it is still corrupting memory starting from vecteur[1]
I'd suggest you to use std::vector<dobule> instead of plain pointers, and filling it with push_back

Upvotes: 2

user405725
user405725

Reputation:

There is definitely a memory problem, and I don't see how using float would fix it. For example, the below line of code allocated only one double and not an array of doubles:

double * vecteur = new double;

Then, you use this vecteur as if it was an array of N elements. That triggers undefined behavior.

To fix it, you would have to allocate an array of as much values as you need. For example, let's say you need 10, then you allocate 10 like this:

double * vecteur = new double[10];

However, given that you don't know the number of elements in advance, you would need to extend the array every time you want to add an element. If you were writing this in C, I would have recommended you to use realloc(). But given that you use C++, just stick with std::vector<double>, it will manage memory automatically. For example:

#include <vector>

int main()
{
    std::vector<double> vecteur; // Use vector to store array of doubles.

    // Add as many elements as you want.
    // Vector will resize itself if/when needed.
    vecteur.push_back(.1);
    vecteur.push_back(.2);
    vecteur.push_back(.3);
    vecteur.push_back(.4);
    vecteur.push_back(.5);
}

Hope it helps. Good Luck!

Upvotes: 7

Related Questions