views:

609

answers:

3

So I have been developing a polynomial class where a user inputs: 1x^0 + 2x^1 + 3x^2... and 1,2,3 (the coefficients) are stored in an int array

My overloaded + and - functions work, however, * doesnt work. No matter the input, it always shows -842150450
when is should be (5x^0 + x^1) * (-3x^0 + x^1) = -15x^0 + 2x^1 + 1x^2
or (x+5)(x-3) = x^2 +2x - 15

I'm using the overloaded * function like : Polynomial multiply = one * two;
Im guessing the problem is strtol(p, &endptr, 10) since it uses a long int, however, adding and subtracting works perfectly

My constructor

Polynomial::Polynomial(char *s)
{
    char *string;
    string = new char [strlen(s) + 1];
    int length = strlen(string);
    strcpy(string, s);

    char *copy;
    copy = new char [length];
    strcpy(copy, string);

    char *p = strtok(string, "  +-");
    counter = 0;
    while (p) 
    {
        p = strtok(NULL, "  +-");
     counter++;
    }

    coefficient = new int[counter];

    p = strtok(copy, "  +");
    int a = 0;
    while (p)
    {
     long int coeff;
     char *endptr;
        coeff = strtol(p, &endptr, 10); //stops at first non number
     if (*p == 'x')
        coeff = 1;

     coefficient[a] = coeff;
     p = strtok(NULL, "  +");
     a++;
    }
}

and the overloaded * function

Polynomial Polynomial::operator * (const Polynomial &right)
{
    Polynomial temp;

    //make coefficient array
    int count = (counter + right.counter) - 1;
    temp.counter = count;
    temp.coefficient = new int [count];
    for (int i = 0; i < counter; i++)
    {
     for (int j = 0; j < right.counter; j++)
      temp.coefficient[i+j] += coefficient[i] * right.coefficient[j];
    }
    return temp;
}

And heres my entire code: http://pastie.org/721143

+7  A: 

You don't appear to initialise the temp.coefficient[i+j] to zero in your operator * ().

temp.coefficient = new int [count];
std::memset (temp.coefficient, 0, count * sizeof(int));
Dave Hinton
An easier way to do this in one statement is `new int[count]()`, which will default-initialize every element.
Pavel Minaev
Thanks, this solved it. Why did I have to add std:memset...
Raptrex
Please look at my answer, your code is not "safe" to work everywhere even with this fix.
Earlz
@Raptrex, it's because `new int[N]` does not fill the array with zeroes. All elements of the array have indeterminate initial values. `memset` will set them all to `0`, or you can use `new int[N](0) or new int[N]()` to do the same thing.
Pavel Minaev
@earlz: I can't see your answer anywhere...
Dave Hinton
+1  A: 

Does

temp.coefficient = new int [count];

give you an array of zeroes?

Otherwise in your for loop you're adding stuff to garbage.

John at CashCommons
+5  A: 

Convert -842150450 to hex to find back one of the magic values used in the CRT in the debug build. That helps finding the bug in your code:

    temp.coefficient = new int [count];
    // Must initialize the memory
    for (int ix = 0; ix < count; ++ix) temp.coefficient[ix] = 0;

There are plenty other bugz btw, good luck fixing them.

Hans Passant
Nice link. +1 for you
Platinum Azure
That's a pretty inefficient way to initialize an array. Use a `memset` instead, or an explicit initializer in `new[]`.
Pavel Minaev
Take a look at the generated code for this loop in the Release build.
Hans Passant
You should still use `memset`, `std::fill`, or best of all `new[]()`. Those functions are standard and easier to read than a hand-written loop.
GMan