adding two structs of integers -> 600 POINTS

DRLski
DRLski used Ask the Experts™
on
I have included links to a class and header file, BigInt.C and BigInt.h.  The class stores integer digits dynamically where basically I input two integers, ie: 999999 and 1
This class adds those two digits up and returns the sum while using structure of dynamic integers to store the digits.  I have already gotten everything working but my "operator+" function is WAY too long and messy and I can't figure out a way to make it shorter but still effective.  Could anyone help me here? I'm going to be including a total of 600 points for whoever helps me out here, I can only give 500 points right now but once this is solved I will set another thread up so I can give that person another 100 points.  If it's a significant improvement on that function than I may include even more points.
The class and header file is here:
http://bsdadmins.net/BigInt.h
http://bsdadmins.net/BigInt.C

Plz help.
Comment
Watch Question

Do more with

Expert Office
EXPERT OFFICE® is a registered trademark of EXPERTS EXCHANGE®
Would you mind posting a source file that demonstrates the abilities of this class?

Commented:
I recommend you use am std::vector instead of your struct Elem.
If you use an std::vector, you can let the build in type (std::vector) do most of the work as far as creating, storing, deleteing, and keeping track of the quantity of Elem/integers.

Commented:
FYI:
You should always avoid placing std into the global namespace in a header file.
Learn Ruby Fundamentals

This course will introduce you to Ruby, as well as teach you about classes, methods, variables, data structures, loops, enumerable methods, and finishing touches.

Author

Commented:
http://bsdadmins.net/main.C

I still need to work on the exception part of it but I'm not worried about that right now.  I don't want to use a predefined class, I need to make a custom one which is why I'm making this stuct the way it is.

thx

Author

Commented:
I'm basically just looking for a compressed version of my code, something more simplified, because my function that I made is doing way too much work, no function should be that long

Commented:
Is this a homework question?
Why does it need to be a custom one?

Author

Commented:
I would just prefer it to be my custom one
Commented:
BigInt BigInt::add(const BigInt &rhs)
{    
     vector<int> i1, i2, isum;
     
     Elem *p1 = this->head;
     Elem *p2 = rhs.head;
     
     for (p1 = this->head; p1 != NULL; p1 = p1->next)
          i1.push_back(p1->value);
     
     for (p2 = rhs.head; p2 != NULL; p2 = p2->next)
          i2.push_back(p2->value);
     
     int count1   = i1.size();
     int count2   = i2.size();
     int countmax = __max(count1, count2);
     
     int value;
     int exceed = 0;
     
     for (int i = 0; i < countmax; i++)
     {
          value = 0;
         
          if (count1 > 0)
               value  = i1[--count1];
         
          if (count2 > 0)
               value += i2[--count2];
         
          value += exceed;
          exceed = value / 10;
          isum.push_back(value % 10);
     }
     
     if (exceed > 0)
          isum.push_back(exceed);
     
     Elem *psum, *p;
     BigInt bigSum;
     bigSum.head = psum = p = NULL;
     
     for (int j = isum.size() - 1; j >= 0; j--)
     {
          psum = new Elem;
          psum->value = isum[j];
          psum->next = NULL;
          if (NULL == p)
          {
               bigSum.head  =  p = psum;
          }
          else
          {
               p->next  = psum;
               p = p->next;
          }
     }
     
     return bigSum;
     
}

You have big problems with memory leaks and it is not the best way to implement bigints. You could use vector<char> for example                    

Commented:
besides, what is the purpose of the 'count' member?

Author

Commented:
I'm gonna test that code out and go through it to see if I can understand what you did.  

The count member is just to find out the length of the integer so I can make it look all pretty on output

Author

Commented:
stupid question...to using vector would I just do

using std::vector;

?

Author

Commented:
nevermind, stupid question

Author

Commented:
BigInt.C:329: implicit declaration of function `int __max(...)

ideas? I'm not familiar with that vector class

Author

Commented:
BigInt.C:329: implicit declaration of function `int __max(...)

ideas? I'm not familiar with that vector class

Commented:
DRLski,
There are examples of BigInt type class that have already been created on the web.
If you which to reinvent the wheel for the porpose of learning, then using a safer, and more C++ method would be your best bet.

The Elem structure you're using is not safe, and it has memory leaks, as previous expert stated.

IMHO, this is not really teaching you anything, other then you should avoid creating your own link list, and use the standard containers instead, which are safer, and have proven and tested code.

Author

Commented:
Kimpan, that did work somewhat, but if I put in a number like 9999999 and add 1 it gives me a sum of 10

Author

Commented:
nevermind, my mistake, when I deleted the __ from max I also deleted max iteslef, it works fine, I'll give you the points right now, thx.

Dave

Author

Commented:
thx for the help, will be posting the rest of the points to another thread in a sec

Commented:
I have a nicer solution to the last loop part. It was kinda sloppy:

replace this:

 Elem *psum, *p;
    BigInt bigSum;
    bigSum.head = psum = p = NULL;
   
    for (int j = isum.size() - 1; j >= 0; j--)
    {
         psum = new Elem;
         psum->value = isum[j];
         psum->next = NULL;
         if (NULL == p)
         {
              bigSum.head  =  p = psum;
         }
         else
         {
              p->next  = psum;
              p = p->next;
         }
    }
   
    return bigSum;
   
to

     BigInt bigSum;
     Elem sum = {0};
     Elem *psum   = &sum;

     while (countmax > 0)
     {
          // create new value
          psum->next  = new Elem;
          psum        = psum->next;
          psum->value = isum[--countmax];
          psum->next  = NULL;
     }

     bigSum.head = sum.next;

     return bigSum;
     
}  

Author

Commented:
thankyou

Author

Commented:
could you tell me what push_back does?

Commented:
I forgot one statement for the nicer loop solution

add

countmax = isum.size();

before

while (countmax > 0)

or it won't work when overflow occurs on the 'highest' position.

999 + 1 would result in '000' and not '1000'

without update of countmax

Author

Commented:
could you tell me what push_back does?

Commented:
it adds the value at the very end of the vector

vector<int> vec;

vec.push_back(1);
vec.push_back(5);
vec.push_back(2);

vec[0] contains 1
vec[1] contains 5
vec[2] contains 2

and as you already know, elements added in a STL vector is contigous in memory. That means you can treat it as an array.

int *a = &vec[0];

a[0] == vec[0]
a[1] == vec[1]
a[2] == vec[2]

Commented:
in fact, use STL vector instead of array. I've shown you how to go from vector to array above. I will show how to go from array to vector to complete the picture.

int a[3] = {1, 5, 2};

vector<int> vec;

CopyMemory(&vec[0], a, sizeof(a));

Commented:
oh forgot again


int a[3] = {1, 5, 2};

vector<int> vec;

vec.resize(sizeof(a) / sizeof(int));

CopyMemory(&vec[0], a, sizeof(a));

Commented:
A more STL method:

int a[3] = {1, 5, 2};
vector<int> vec(&a[0], &a[sizeof(a)/sizeof(int)]);

The above method is more efficient and less code..

Commented:
Sorry Kimpan, but I didn't unsubcribed from this question in time, so I'm gettin hit with a whole bunch of email notifications.
So I firgured I through my two cents into it. :-)

Commented:
FYI:
If you know the required size for your vector, you should defined the size in the constructor instead of resize.
Example:
int a[3] = {1, 5, 2};

vector<int> vec(sizeof(a) / sizeof(int));
std::copy(&a[0], &a[sizeof(a)/sizeof(int)], &vec[0]);

And since you're dealing with STL code, you might as well use STL functions instead of a windows API function, which is not portable, and can lead to logic errors.

Commented:
you are right Axter

Commented:
which way is the best one to copy the contents of an array to an already existing vector?

std::vector<int> vec;

...

int a[3] = {1, 5, 3};

// wanna copy a to vec

Commented:
>>which way is the best one to copy the contents of an array to an already existing vector?


Then you have to do resize, and use the std::copy function.

Example:
std::vector<int> vec;
int a[3] = {1, 5, 3};
vec.resize(sizeof(a) / sizeof(a[0]));
std::copy(&a[0], &a[sizeof(a)/sizeof(a[0])], &vec[0]);

Do more with

Expert Office
Submit tech questions to Ask the Experts™ at any time to receive solutions, advice, and new ideas from leading industry professionals.

Start 7-Day Free Trial