-4

I am trying to implement Big Number in c++ with strings. First I am doing operator overloading for '+' operator, addition for digit length > 3 is coming correct but for less than 3 is giving garbage value after result. I am not understanding why this is happening. Here is my code.

#include<iostream>
#include<cstring>
#define SIZE 100
using namespace std;
class BigNumber
{
    char *chNumber;
    long nLength,nMaxLength;
    public :
        BigNumber()
        {
            nLength = nMaxLength = 0;
            Init();
        }
        BigNumber(long length)
        {
            nLength = length;
            nMaxLength = length;
            Init();
        }
        BigNumber(const char *str,long length)
        {
            nLength = strlen(str);
            nMaxLength = length;            
            Init();
            memset(chNumber,0,sizeof(char));
            strncpy(chNumber,str,length);
        }
        /*** copy constructor **/
        BigNumber(const BigNumber &source)
        {
            nLength = strlen(source.chNumber);
            nMaxLength = source.nMaxLength;
            if(source.chNumber)
            {
                Init();
                strncpy(chNumber,source.chNumber,nLength);
            }
            else
                chNumber = 0;
        }
        void Init()
        {
            chNumber = new char[nMaxLength + 5];
            memset(chNumber,0,sizeof(char));
        }
        ~BigNumber()
        {
            delete[] chNumber;
            chNumber = 0;
            nLength = 0;
        }
        char* getNumber()
        {
            return chNumber;
        }
        const long size() const
        {
            return nLength;
        }
        const long capacity() const
        {
            return nMaxLength;
        }
        friend long maxsize(BigNumber &obj1,BigNumber &obj2) 
        {
            return (obj1.size() > obj2.size()) ? obj1.size():obj2.size();
        }
        friend ostream& operator<<(ostream &out,BigNumber &obj)
        {
            //out<<"String is "<<obj.chNumber<<" of length "<<obj.nLength
            //  <<" and max length allocated is "<<obj.nMaxLength;
            out<<obj.chNumber;
            return out;
        }

        friend istream& operator>>(istream &in,BigNumber &obj)
        {
            obj.Init();
            in>>obj.chNumber;
            obj.nLength = strlen(obj.chNumber);
            return in;
        }   
        BigNumber &operator=(const BigNumber &obj)
        {
            if(this == &obj)
            {
                return *this;
            }
            delete[] chNumber;
            chNumber = 0;
            nLength = strlen(obj.chNumber);
            Init();
            strncpy(chNumber,obj.chNumber,nLength);
            return *this;
        }
        void operator=(char * str)
        {
            nLength = strlen(str);
            Init();
            strncpy(chNumber,str,nLength);
        }
        friend BigNumber reverse(const BigNumber &obj1)
        {
            long length = obj1.size();
            int m=0;
            BigNumber obj2(length+5);
            for(int i=length-1;i>=0;--i)
            {
                obj2.chNumber[m++]=obj1.chNumber[i];
            }
            obj2.chNumber[m]='\0';
            //cout<<obj2.chNumber<<endl;
            obj2.nLength = m;
            return obj2;
        }
        friend BigNumber operator+(BigNumber &obj1,BigNumber &obj2)
        {
            long newLength = maxsize(obj1,obj2);
            BigNumber obj3(newLength + 5);
            int length1 = obj1.size();
            int length2 = obj2.size();
            int i,j,carry=0,num,m=0;
            for(i=length1-1,j=length2-1; i>=0 || j>=0 ; --i,--j)
            {
                if(i>=0 && j>=0)
                {
                    num = (obj1.chNumber[i]-'0') + (obj2.chNumber[j]-'0') + carry;
                }
                else if(i>=0)
                {
                    num = obj1.chNumber[i] - '0' + carry;
                }
                else if(j>=0)
                {
                    num = obj2.chNumber[j] - '0' + carry;
                }
                carry = num/10;
                num = num%10;
                obj3.chNumber[m++] = num+'0';
            }
            obj3.chNumber[m]='\0';
            obj3.nLength = m; 
            BigNumber obj4 = reverse(obj3);
            cout<<obj4<<endl;
            return reverse(obj3);
        }
};

void test_addition()
{
    BigNumber n1("42",3),n2("1",2),n3;
    n3 = n1 + n2;
    cout<<n3<<endl;
    n1 = "123";
    n2 = "345";
    n3 = n1 + n2;
    cout<<n3<<endl;
}
int main()
{
    test_addition();
    return 0;
}

Before returning from operator+() function, I am printing sum its giving correct output and after returning I am printing is giving junk values after result.

Thank You

Remy Lebeau
  • 555,201
  • 31
  • 458
  • 770
avinashse
  • 1,440
  • 4
  • 30
  • 55
  • 3
    Find a case where the sum is wrong and step through the addition operator with a debugger. Alternatively, add print statements in that function to help you see what's going wrong. If there's code behavior that you are unable to explain, please post it here. – Pradhan Dec 28 '14 at 06:17
  • 1
    I think you need to look at your constructors, and the assignment operator from `char *`. I don't think you're setting the lengths consistently, and you've probably got problems arising from that. – Jonathan Leffler Dec 28 '14 at 06:19
  • @Pradhan Perhaps, you should give some useful comment regarding the problem not the debugging tools, in no case sum is getting wrong, you can check that in the operator+(), print reverse(obj3) before returning, perhaps you haven't checked my code where its wrong. – avinashse Dec 28 '14 at 07:21
  • @avinashse Of course I haven't done a detailed analysis of your code - I did a perfunctory read and realised I needed more data to diagnose it. Note that while people on SO do try to help, they are unlikely to pore through every random code snippet thrown their way. Also, how about reading SO's guidelines on [asking good questions](http://stackoverflow.com/help/how-to-ask) first? After that, feel free to come back here and comment on the usefulness of this comment. – Pradhan Dec 28 '14 at 07:28
  • @JonathanLeffler: Thank you for the comment, My problem is when I do BigNumber obj4 = reverse(obj3);cout< – avinashse Dec 28 '14 at 07:28
  • @Pradhan : Ok.. I did all that print stuffs, not working, checked every thing which I know and now added the problem which I am getting, I will be thankful if you give your time to resolve my issue. – avinashse Dec 28 '14 at 07:34
  • 1
    You don't have a copy constructor defined and the default one gets used resulting in a shallow copy. Write a `BigNumber(const BigNUmber&)` which looks like your `operator=` and that should fix your issue. – Pradhan Dec 28 '14 at 07:37
  • @Pradhan I added copy constructor, but problem is still there. – avinashse Dec 28 '14 at 07:56
  • Ok. So this is when you run a debugger and examine the internals of the result of `reverse(obj3)` and the internals of `n3` after the end of the buggy addition. – Pradhan Dec 28 '14 at 08:00
  • @Pradhan yes, problem is before returning i am getting correct value and after returning its wrong and one more thing this code is running fine for more than 3 digits summation – avinashse Dec 28 '14 at 09:12
  • Where do you set the terminating `'\0'` in your copy constructor? – Marc Glisse Dec 28 '14 at 10:07
  • `char *chNumber;` <- I have bad news for you: this is not a string, it's an ancient abomination from the C times. – Griwes Dec 28 '14 at 20:18

1 Answers1

2

There are some problems in your implementation, namely:

  1. bad memory management
  2. a missing copy constructor
  3. operator+ is not accounting for a non-zero carry after the loop exits, or accounting for the possibility that the sum may have a longer length then the two input values.

Try this instead:

#include <iostream>
#include <cstring>
#include <iomanip>

#define SIZE 100

class BigNumber
{
private:
    char *chNumber;
    long nLength, nMaxLength;

    public :
        BigNumber()
        {
            nLength = nMaxLength = 0;
            Init();
        }

        BigNumber(long initialCapacity)
        {
            nLength = 0;
            nMaxLength = initialCapacity;
            Init();
        }

        BigNumber(const char *str)
        {
            nLength = nMaxLength = strlen(str);
            Init();
            memcpy(chNumber, str, nLength);
        }

        BigNumber(const BigNumber &source)
        {
            nLength = source.nLength;
            nMaxLength = source.nMaxLength;
            Init();
            memcpy(chNumber, source.chNumber, nLength);
        }

        void Init()
        {
            chNumber = new char[nMaxLength + 5];
            memset(chNumber, 0, nMaxLength + 5);
        }

        ~BigNumber()
        {
            delete[] chNumber;
            chNumber = 0;
            nLength = nMaxLength = 0;
        }

        const char* getNumber() const
        {
            return chNumber;
        }

        const long size() const
        {
            return nLength;
        }

        const long capacity() const
        {
            return nMaxLength;
        }

        friend long maxsize(const BigNumber &obj1, const BigNumber &obj2)
        {
            return (obj1.size() > obj2.size()) ? obj1.size(): obj2.size();
        }

        friend ostream& operator<<(ostream &out, const BigNumber &obj)
        {
            //out<<"String is "<<obj.chNumber<<" of length "<<obj.nLength
            //  <<" and max length allocated is "<<obj.nMaxLength;
            out << obj.chNumber;
            return out;
        }

        friend istream& operator>>(istream &in, BigNumber &obj)
        {
            char tmp[SIZE+1];
            memset(tmp, 0, SIZE);
            in >> setw(SIZE) >> tmp;
            obj = tmp;
            return in;
        }

        BigNumber &operator=(const BigNumber &obj)
        {
            if (this != &obj)
            {
                delete[] chNumber;
                chNumber = 0;
                nLength = obj.nLength;
                nMaxLength = obj.nMaxLength;
                Init();
                memcpy(chNumber, obj.chNumber, nLength);
            }
            return *this;
        }

        BigNumber& operator=(const char* str)
        {
            delete[] chNumber;
            chNumber = 0;
            nLength = nMaxLength = strlen(str);
            Init();
            memcpy(chNumber, str, nLength);
        }

        friend BigNumber reverse(const BigNumber &obj1)
        {
            long length = obj1.size();
            int m = 0;
            BigNumber obj2(length);
            for(int i = length-1; i >= 0; --i)
            {
                obj2.chNumber[m++] = obj1.chNumber[i];
            }
            obj2.chNumber[m] = '\0';
            //cout << obj2.chNumber << endl;
            obj2.nLength = m;
            return obj2;
        }

        friend BigNumber operator+(const BigNumber &obj1, const BigNumber &obj2)
        {
            long newCapacity = maxsize(obj1, obj2) + 1;
            BigNumber obj3(newCapacity);
            int length1 = obj1.size();
            int length2 = obj2.size();
            int carry = 0, num, m = 0;
            for(int i = length1-1, j = length2-1; (i >= 0) || (j >= 0); --i, --j)
            {
                if ((i >= 0) && (j >= 0))
                {
                    num = (obj1.chNumber[i]-'0') + (obj2.chNumber[j]-'0') + carry;
                }
                else if (i >= 0)
                {
                    num = obj1.chNumber[i] - '0' + carry;
                }
                else
                {
                    num = obj2.chNumber[j] - '0' + carry;
                }
                carry = num/10;
                num %= 10;
                obj3.chNumber[m++] = num+'0';
            }
            if (carry != 0)
                obj3.chNumber[m++] = carry+'0';
            obj3.chNumber[m]='\0';
            obj3.nLength = m;
            return reverse(obj3);
        }
};

With that said, I strongly suggest you change your internal buffer to use a std::string instead of a char[] array, and also to use standard STL functions instead of writing your own. This will simplify the implementation a little bit:

#include <iostream>
#include <cstring>
#include <iomanip>
#include <algorithm>

#define SIZE 100

class BigNumber
{
private:
    std::string chNumber;

public:
    BigNumber()
    {
    }

    BigNumber(const BigNumber &src)
        : chNumber(src.chNumber)
    {
    }

    BigNumber(long initialCapacity)
    {
        chNumber.reserve(initialCapacity);
    }

    BigNumber(const char *str)
        : chNumber(str)
    {
    }

    const char* getNumber() const
    {
        return chNumber.c_str();
    }

    const long size() const
    {
        return chNumber.size();
    }

    const long capacity() const
    {
        return chNumber.capacity();
    }

    friend ostream& operator<<(ostream &out, const BigNumber &obj)
    {
        //out<<"String is " << obj.chNumber << " of length " << obj.size()
        //  << " and max length allocated is " << obj.capacity();
        out << obj.chNumber.c_str();
        return out;
    }

    friend istream& operator>>(istream &in, BigNumber &obj)
    {
        char tmp[SIZE+1];
        memset(tmp, 0, SIZE);
        in >> std::setw(SIZE) >> tmp;
        obj = tmp;
        return in;
    }

    BigNumber& operator=(const BigNumber &obj)
    {
        chNumber = obj.chNumber;
        return *this;
    }

    friend BigNumber operator+(const BigNumber &obj1, const BigNumber &obj2)
    {
        BigNumber obj3(std::max(obj1.size(), obj2.size())+1);
        int length1 = obj1.size();
        int length2 = obj2.size();
        int carry=0, num;
        for (int i = length1-1, j = length2-1; (i >=0) || (j >= 0); --i,--j)
        {
            if ((i >= 0) && (j >= 0))
            {
                num = (obj1.chNumber[i] - '0') + (obj2.chNumber[j] - '0') + carry;
            }
            else if (i >= 0)
            {
                num = (obj1.chNumber[i] - '0') + carry;
            }
            else
            {
                num = (obj2.chNumber[j] - '0') + carry;
            }
            carry = num / 10;
            num %= 10;
            obj3.chNumber += ('0'+num);
        }
        if (carry != 0)
            obj3.chNumber += ('0'+carry);
        std::reverse(obj3.chNumber.begin(), obj3.chNumber.end());
        return obj3;
    }
};

void test_addition()
{
    BigNumber n1("42"), n2("1"), n3;
    n3 = n1 + n2;
    std::cout << n3 << std::endl;
    n1 = "123";
    n2 = "345";
    n3 = n1 + n2;
    std::cout << n3 << std::endl;
}

int main()
{
    test_addition();
    return 0;
}
Remy Lebeau
  • 555,201
  • 31
  • 458
  • 770
  • Thank you for the reply, I am getting your point but I when I do `BigNumber obj4 = reverse(obj3);cout< – avinashse Dec 28 '14 at 07:27
  • I tested the code I posting, and it worked fine for me. The return value of `operator+` is correct. Notice that I removed the `BigNumber::reverse()` method and replaced it with `std::reverse()` inside of `operator+`. If you are still using `reverse(obj3)`, then you are not using the code I gave you. That being said, in your original code, you are missing a copy constructor that copies data from one `BigNumber` to another, which is likely contributing to your memory management problems. – Remy Lebeau Dec 28 '14 at 07:40
  • Your code is running fine and that is the last option,but I dont want to use string as this will increase the processing time and I want to figure out why my code is not working. I added copy constructor. – avinashse Dec 28 '14 at 08:58
  • The problem is before returning i am getting correct value and after returning its wrong and one more thing my code is running fine for more than 3 digits summation – avinashse Dec 28 '14 at 09:13
  • I ran your updated code as-is (after fixing 1 typo in it - `obj3.n/Length = m` should be `obj3.nLength = m`) and it works fine for me. Before and after the return, the output is the same. If you are still having the problem, it might be an issue with your compiler, not the code. – Remy Lebeau Dec 28 '14 at 17:49