Overloaded function not outputting correct answer

46 views Asked by At

I am attempting to create an overloaded contructor that will take in integer and add them with the byteAdd function. However; whenever I compile the output is always the default constructor rather than the sum of two overloaded constructors. I have used the clear function in the setValue() function however the error still persists. Any clue as to why the issue is persisting in my code?

#pragma once
#ifndef MATH
#define MATH

#include <string>
#include <vector>
#include <algorithm>
#include <iostream>


using std::string;
using std::vector;

class Math {

private:

    int num1{};
    int bitsToInt() const;

public:
    void setValue(int value);
    int at(int index) const;
    string toString() const;
    int toInt() const;

    struct Bits {
        int value;
        Bits(int v) : value(v) {}
    };
    
    vector<Bits>bits; 
    Math byteAdd(const Math& f);

    //Constructor
    Math();//Default constructor
    Math(int val); //This is an overloaded constr
    
};

#endif // !MATH


//Math.cpp
#include <iostream>
#include <cstdlib>
#include "Math.h"


using namespace std;

//Default Math constructor
Math::Math() {
    this->setValue(0);
}

//Overloaded Constructor
Math::Math(int val) {
    this->setValue(val);
    for (int i = 0; i < 8; ++i) {
        this->bits.push_back(Bits{ (val >> i) & 1 });
    }
}

void Math::setValue(int value) {
    this->num1 = value;
    for (int i = 0; i < 8; ++i) {
        this->bits.push_back(Bits{ (value >> i) & 1 });
    }
}

int Math::bitsToInt() const {
    int val1 = 0;
    for (int i = 0; i < 8; i++) {
        val1 |= (bits[i].value << i);
    }
    return val1;
}

int Math::at(int index) const {
    return bits[index].value;
}

std::string Math::toString() const {
    std::string str;
    for (int i = 7; i >= 0; --i) {
        str += std::to_string(bits[i].value);
    }
    return str;
}

int Math::toInt() const {
    return bitsToInt();
}

Math Math::byteAdd(const Math& f) {
    Math tmp;
    tmp.num1 = num1; // Assigning num1 of current object to num1 of tmp 
    int carry = 0;

    for (int i = 0; i < 8; ++i) {
        int s = bits[i].value + f.bits[i].value + carry; // Accessing bits of f
        tmp.bits.push_back(Bits{ s % 2 }); // Using push_back to add new Bits
        carry = s / 2;
    }
    return tmp;
}


//Main.cpp
#include <iostream>
#include "Math.h"

using namespace std;



int main() {

    Math obj1, obj2, obj4, obj5(5), obj6(5);
    Math obj3 = obj1.byteAdd(obj2);
    Math obj7 = obj5.byteAdd(obj6);

    cout << "Int: " << obj7.toInt() << endl;
    cout << "String " << obj7.toString() << endl;

    return 0;

}

I want the output to be the sum of, in this case, obj5(5) and obj6(5), which would result in 10 and the string outputting 0001010. However I am getting 0, which is the initial setValue when I run the program.

Output of the program with the overloaded Functions

1

There are 1 answers

0
463035818_is_not_an_ai On

You are looking at the wrong elements of the vector.

You should use a debugger to step through the code line by line. You will then see that here:

Math Math::byteAdd(const Math& f) {
    Math tmp;
    tmp.num1 = num1; // Assigning num1 of current object to num1 of tmp 
    int carry = 0;

    for (int i = 0; i < 8; ++i) {
        int s = bits[i].value + f.bits[i].value + carry; // Accessing bits of f
        tmp.bits.push_back(Bits{ s % 2 }); // Using push_back to add new Bits
    // ...

Math tmp calls the default constructor. The default constructor calls this->setValue(0);. setValue pushes 8 elements to the vector. Then in the loop you push another 8 elements in the vector. The code that follows only ever considers the first 8 elements (that contain 0, the next 8 elements that contain the correct value are not accessed after they have been pushed).

Similar issue with Math(int val). It calls setValue(val) to push 8 elements to the vector then in the constructors body you push 8 more elements to the vector. This issue is just not as severe because you push twice the same elements.

Do not use a vector when you always need 8 elements. Replace the vector with a std::array<int,8> and instead of pushing elements, simply access them. This should fix the problem with your code.

(Note: Bits is just an int. It brings no advantage compared to using a plain int.)