BigInt mutliplication and subtraction error

Question

I'm assignment I have to create a calculator that works with big integers up to 256 characters in length. Im getting error being unable to multiple as well as weird problems how some subtraction numbers work such as 23456 - 13141= 10315 however any other subtraction such as 6 - 5 doesn't work. im very stumped to why this is.

Full code below

#include <iostream>   
#include <string> 
#include "Bigint.h" 

using namespace std; 

Bigint::Bigint() 
{
    for (int i = DIGITS-1; i >= 0; --i) {
        digits_[i] = 0;
    }
    radix = 10;
}

Bigint::Bigint(int r)  
{
    for (int i = DIGITS-1; i >= 0; --i) {
        digits_[i] = 0;
    }
    radix = r;
}

ostream& operator<< (ostream& out, const Bigint& n) 
{
    string s = "";
    int leadingZeros = 0;

    for (int i = DIGITS-1; i >= 0 ; --i) { 

        if(n.digits_[i] != 0){      
            leadingZeros = 1;
        }

        if(leadingZeros == 1){
            s += char(n.digits_[i] + '0'); 
        }
    }
    return out << s;
}

istream& operator>> (istream& in, Bigint& n)
{

    string s;
    if (in >> s) {

        int length = s.length(); 
        int i;
        for (i = 0; i < DIGITS && i < length; ++i) {
            n.digits_[i] = int(s[length-1 - i] - '0'); 
        }
    }
    return in;
}

Bigint operator+ (const Bigint& n1, const Bigint& n2)
{
    Bigint add;
    int carry = 0, sum = 0;

    for(int i=0; i < DIGITS; ++i){ 
        sum = n1.digits_[i] + n2.digits_[i] + carry; 
        add.digits_[i] = sum % 10;
        carry = sum / 10;
    }
    return add;
}

Bigint operator- (const Bigint& n1, const Bigint& n2) 
{
    Bigint sub;
    int carry = 0; 
    for (int i=0; i< DIGITS; i++) 
    { 

        int val = n1.digits_[i] - n2.digits_[i] -carry;  

        if (val < 0) 
        { 
            val += 10; 
            carry = 1; 
        } 
        else
            carry = 0; 

        sub.digits_[i] = val;
    } 
    return sub;
}

Bigint Bigint :: multiplyBigint(const Bigint& num, const int n, int count){ 

    Bigint result;
    int carry =0;

    for(int i=0; i< DIGITS; ++i){
        int val = (num.digits_[i] * n) + carry;
        result.digits_[i+count] = val % 10;
        carry = val / 10; 
    }

    return result;
}

Bigint operator* (const Bigint& n1, const Bigint& n2)
{
    Bigint multiply;
    Bigint temp;

    int count =0;

    for(int i=0; i< DIGITS; ++i){
        temp = Bigint :: multiplyBigint(n1, n2.digits_[i], count);
        count++;
        multiply = multiply + temp;
    }

    return multiply;
}

Bigint operator/ (const Bigint& n1, const Bigint& n2)
{
    Bigint divide;
    Bigint temp = n1;

    while(temp > n2){
        divide = divide + 1;
        temp = temp - n2;
    }

    return divide; 
}

Bigint operator+ (const Bigint& n1, int n2)
{
    Bigint add;
    int carry = 0, sum = 0;

    for(int i=0; i < DIGITS; ++i){
        sum = n1.digits_[i] + (n2 % 10) + carry;
        n2 = n2 / 10;
        add.digits_[i] = sum % 10;
        carry = sum / 10;
    }
    return add;
}

bool operator> (const Bigint& n1, const Bigint& n2){

    for(int i= DIGITS - 1; i >= 0; --i){
        if(n1.digits_[i] > n2.digits_[i])
            return true;
    }
    return false;
}

Header file

#ifndef BIGINT_H_ //This checks to whether the given token has been defined somewhere else in the file.
#define BIGINT_H_

#define DIGITS 256 //increases the array size to 256 digits.

class Bigint
{
  public: // this makes the members of the public accessible from anywhere in the project.

    /**
     * Creates a Bigint initialised to 0.
     */
    Bigint(); 
    Bigint(int r);

    /**
     * Inserts n into stream or extracts n from stream.
     */
    friend std::ostream& operator<< (std::ostream &out, const Bigint& n);
    friend std::istream& operator>> (std::istream &in, Bigint& n);

    /**
     * Returns the sum, difference, product, or quotient of n1 and n2 and compares them.
     */
    friend Bigint operator+ (const Bigint& n1, const Bigint& n2);
    friend Bigint operator- (const Bigint& n1, const Bigint& n2);
    friend Bigint operator* (const Bigint& n1, const Bigint& n2);
    friend Bigint operator/ (const Bigint& n1, const Bigint& n2);

    friend Bigint operator+ (const Bigint& n1, int n2);
    friend bool operator> (const Bigint& n1, const Bigint& n2);

  private: //making this only accessible within other members of this same class.
    int digits_[DIGITS];
    int radix;
    static Bigint multiplyBigint(const Bigint& num, const int n, int count);
};

#endif // BIGINT_H_

Main.cpp

#include <iostream> //provides basic input and output services such as char.
#include "Bigint.h" //provides access and link to header file.

using namespace std;

int findRadix(string value){ //uses the Radix sort alogrithm to sort each value.

    if(value.length() == 3){
        if(value[0] == '-' && value[1] == 'r'){
            if(value[2] >= '2' && value[2] <= '9')
                return value[2] - '0';
            else if(value[2] >= 'A' && value[2] <= 'Z')
                return (value[2] - 'A') + 10;
        }
    }

    return 10;
}

int main(int argc, char *argv[]) 
{
    int radix;

    if(argc ==2){
        radix = findRadix(argv[1]);
    }


    Bigint n1(radix), n2(radix); //This compares n1 and n2 to each other to give a result.
    char op;

    while (cin >> n1 >> op >> n2) {
        switch (op) {
        case '+' :
            cout << n1 + n2 << endl;
            break;
        case '-' :
            cout << n1 - n2 << endl;
            break;
        case '*' :
            cout << n1 * n2 << endl;
            break;
        case '/' :
            cout << n1 / n2 << endl;
            break;
        }
    }

    return 0;
}

Answer 1

It seems I found your problem - you are not cleaning after yourself. That is, each time you enter a new number it also uses some of the old number.

There are several possible solutions to this problem. The simplest one (with some code fix-ups) is as follows:

Create a method clear somewhere in your class:

void Bigint::clear()
{
    for (int i = DIGITS-1; i >= 0; --i)
        digits_[i] = 0;
}

Erase the default constructor (this is optional but he's redundant), and change the other one to this:

Bigint::Bigint(int radix = 10) : radix{radix}
{
    clear();
}

And add the following at the start of your input operator:

istream& operator>> (istream& in, Bigint& n)
{
    clear(); //This is the important one!!!!

    string s;
    if (in >> s) {
    ...
    ...
}

There are other possible solutions as well:

• You could add a member int length; to your class. You already have this length in your input operator, you just need to keep it and use it in all your calculations.

• Even better, change your digits_ to std::vector<int> digits_;. This will allow you to get rid of the clear function thus leaving your constructor empty (only with the member initialization). You would just have to call digits_.clear(); at the start of your input operator. And use digits_.push_back(...); instead of fiddling with indexes.

But the most correct solution (though changing to std::vector<int> should be done anyhow) is to just erase your input operator and change your Bigint::Bigint to accept a string. That is:

Bigint::Bigint(std::string s, int radix = 10) : radix{radix}
{
    const int length = s.length();
    for (int i = 0; (i < DIGITS) && (i < length); ++i)
        n.digits_[i] = int(s[length()-1 - i] - '0');
}

After that, in your main, you just do this:

void main(...)
{
    ....

    char op;
    std::string s1, s2;

    while (cin >> s1 >> op >> s2)
    {
        Bigint n1 {s1, radix};
        Bigint n2 {s2, radix};

        switch (op) {
        .....
        }
    }
}

This has the following benefits:

• The strings are always cleared before input. See here: https://en.cppreference.com/w/cpp/string/basic_string/operator_ltltgtgt
• The Bigint is always created fresh.
• You can create Bigints from any string, not just std::cin.

I would strongly consider replacing your digits_ with std::vector<int>. This will have the benefit of accepting number of any size, since the vector can grow as large as you need it. This would also make your string-to-int conversion code much simpler. And make you multiply simpler since you can easily append zeroes at the end and beginning of the vector with a simple operation. And would avoid a lot of other mistakes. Just change it to std::vector<int>! ;-)

Also, please note that using namespace std; is bad practice: Why is "using namespace std;" considered bad practice?