0

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

4
  • 1
    please include the output and expected output in the question. This will help others to reproduce and fix the issue Commented Mar 18, 2024 at 8:29
  • 1
    in Math::Math(int) you first call setValue which pushes 8 elements into the vector, then the constructor pushes 8 more elements to the vector, but you only ever read the first 8 elements. Not sure if this is the only issue, but it is an issue. If the vector has 8 elements always, make it an array. And Bits is just an int. Hence you can make the member a std::array<int,8> bits; Commented Mar 18, 2024 at 8:57
  • @463035818_is_not_an_ai 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 Commented Mar 18, 2024 at 16:37
  • Please read What's the problem with "using namespace std;"? - this is especially bad in a header file; don't ever do that. Commented Mar 18, 2024 at 17:09

1 Answer 1

0

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.)

Sign up to request clarification or add additional context in comments.

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.