Overloading the assignment operator= doesn't work for anything except the implicit object passe-CodePudding

The comparison in the assignment operator works as expected, but when I try to use a temp variable and load then return that, all it returns is the defaults. Debugs show that the values are default 1 x 4, but I'm not sure what to with it to make it fill the temp variables and return them. Everything I've gone through says it should work.

Debug info:

-       poly    {coefficient="2" variable="x" exponent="4" }    Polynomial
        coefficient "2" std::string
        variable    "x" std::string
        exponent    "4" std::string
-       poly2   {coefficient="3" variable="x" exponent="4" }    Polynomial
        coefficient "3" std::string
        variable    "x" std::string
        exponent    "4" std::string
-       poly3   {coefficient="1" variable="x" exponent="4" }    Polynomial
        coefficient "1" std::string
        variable    "x" std::string
        exponent    "4" std::string

main

    // Demonstrating class Polynomial's overloaded stream insertion 
// and stream extraction operators.
#include <iostream>

#include "Polynomial.h"
using namespace std;

int main() {


    Polynomial poly; // create object poly
    Polynomial poly2; // create object poly
    Polynomial poly3; // create object poly
    Polynomial poly4; // create object poly
    Polynomial poly5; // create object poly


    cout << "Enter polynomial number in the form 2x4:" << endl;
    // cin >> phone invokes operator>> by implicitly issuing
    // the non-member function call operator>>(cin, phone)
    cin >> poly;
    cout << "\nThe polynomial number entered was:\n";
    // cout << phone invokes operator<< by implicitly issuing
    // the non-member function call operator<<(cout, phone)
    cout << poly << endl;
    cout << "Enter polynomial number in the form 2x4:" << endl;
    // cin >> phone invokes operator>> by implicitly issuing
    // the non-member function call operator>>(cin, phone)
    cin >> poly2;
    cout << "\nThe polynomial number entered was:\n";
    // cout << phone invokes operator<< by implicitly issuing
    // the non-member function call operator<<(cout, phone)
    cout << "poly2 " << poly2 << endl;
    poly3 = poly   poly2;
    cout << "poly3 " << poly3 << endl;
    poly4 = poly - poly2;
    cout << "poly4 " << poly4 << endl;
    poly5 = poly;
    cout << "poly5 " << poly5 << endl;
  }

Header

#pragma once
#ifndef POLYNOMIAL_H
#define POLYNOMIAL_H

#include <iostream>
#include <string>

class Polynomial {
    friend std::ostream& operator<<(std::ostream&, const Polynomial&);
    friend std::istream& operator>>(std::istream&, Polynomial&);

public:
    // Default Constructor

    Polynomial(std::string coefficient = "1", std::string variable = "x",
        std::string exponent = "4");

    // Copy Constructor
    Polynomial(const Polynomial& copy)
        : coefficient{ copy.coefficient }, variable{ copy.variable },
        exponent{ copy.exponent } {
        std::cout << "Copy Constructor called" << std::endl;
    }

    void setPolynomial(std::string, std::string, std::string);
    Polynomial getPolynomial();
    // addition operator; Polynomial   Polynomial
    Polynomial operator (const Polynomial&) const;
    // subtraction operator; Polynomial - Polynomial
    Polynomial operator-(const Polynomial&) const;
    // assigment operator; Polynomial - Polynomial
    Polynomial operator=( Polynomial&);

private:
    std::string coefficient; // 
    std::string variable;    // 
    std::string exponent;    // 
};

#endif

Polynomial.cpp

// Overloaded stream insertion and stream extraction operators
// for class PhoneNumber.
#include "Polynomial.h"
#include <iomanip>
using namespace std;

// default constructor; conversion constructor that converts
Polynomial::Polynomial(std::string co, std::string va, std::string ex) {}
// Setters
void Polynomial::setPolynomial(std::string co, std::string va, std::string ex) {
    this->coefficient = co;
    this->variable = va;
    this->exponent = ex;
}
// Getters
Polynomial Polynomial::getPolynomial() { return *this; }
// overloaded stream insertion operator; cannot be a member function
// if we would like to invoke it with cout << somePhoneNumber;
ostream& operator<<(ostream& output, const Polynomial& number) {
    output << "Coefficient: " << number.coefficient
        << "\nVariable: " << number.variable
        << "\nExponent: " << number.exponent << "\n"
        << "" << number.coefficient << "" << number.variable << "^"
        << number.exponent << "\n";
    return output; // enables cout << a << b << c;
}

// overloaded stream extraction operator; cannot be a member function
// if we would like to invoke it with cin >> somePhoneNumber;
istream& operator>>(istream& input, Polynomial& number) {
    input >> setw(1) >> number.coefficient; // input area code
    input >> setw(1) >> number.variable;    // input exchange
    input >> setw(1) >> number.exponent;    // input line
    return input;                           // enables cin >> a >> b >> c;
}

// addition operator; Polynomial   Polynomial
// A member function takes an implicit first parameter
Polynomial Polynomial::operator (const Polynomial& op1) const {
    Polynomial temp; // temporary result

    if (this->variable == op1.variable) {
        if (this->exponent == op1.exponent) {
            // Use stoi string to int
            int num1 = stoi(this->coefficient);
            int num2 = stoi(op1.coefficient);
            // use to_string to set coefficient
            std::string s = std::to_string(num1   num2);
            temp.coefficient = s;
            temp.variable = this->variable;
            temp.exponent = this->exponent;
        }
        else {
            std::cout << "Exponents must match\n";
        }
    }
    else {
        std::cout << "Variables must match\n";
    }
    return temp; // return copy of temporary object
}
// substraction operator; Polynomial - Polynomial
// A member function takes an implicit first parameter
Polynomial Polynomial::operator-(const Polynomial& op1) const {
    Polynomial temp; // temporary result

    if (this->variable == op1.variable) {
        if (this->exponent == op1.exponent) {
            // Use stoi string to int
            int num1 = stoi(this->coefficient);
            int num2 = stoi(op1.coefficient);
            // use to_string to set coefficient
            std::string s = std::to_string(num1 - num2);
            temp.coefficient = s;
            temp.variable = this->variable;
            temp.exponent = this->exponent;
        }
        else {
            std::cout << "Exponents must match\n";
        }
    }
    else {
        std::cout << "Variables must match\n";
    }
    return temp; // return copy of temporary object
}

// assignment operator; Polynomial - Polynomial
// A member function takes an implicit first parameter
Polynomial Polynomial::operator=(Polynomial& op1){
    // self assignment guard
    if (this == &op1) {
        return *this;//This returns as expected.
        // 
    } // This should create a new temp, assign the second's info to it and return it.
    // But all it does is return the default constructor values 1x4
    Polynomial temp;
    temp.coefficient = op1.coefficient;
    temp.variable = op1.variable;
    temp.exponent = op1.exponent;
    return temp;
}

CodePudding user response：

Polynomial.cpp:

Polynomial Polynomial::operator=( Polynomial op1)  {
    // self assignment guard
    if (this == &op1) {
        return *this;
        // Do the copy
  }
    Polynomial temp;
    this->coefficient = op1.coefficient;
    this->variable = op1.variable;
    this->exponent = op1.exponent;
    return temp;
}

Polynomial.h

 Polynomial operator=( Polynomial) ;

CodePudding user response：

Your operator= makes no sense. There are two reasonable ways to implement operator=:

Using the copy-and-swap idiom, where the prototype for Polynomial would be:

  Polynomial& operator=(Polynomial other);
//^^^^^^^^^^^ Returns a reference
                        ^^^^^^^^^^ Receives argument by value

or,

"The hard way" (reimplementing stuff swap could do for you, and possibly implementing two different versions, one for copy assignment, and an optional one for move assignment), where the prototypes are:

  Polynomial& operator=(const Polynomial& other);
//^^^^^^^^^^^ Returns a reference
                        ^^^^^^^^^^^^^^^^^ Receives argument by const reference for copying
  Polynomial& operator=(Polynomial&& other);
//^^^^^^^^^^^ Returns a reference
                        ^^^^^^^^^^^^ Receives argument by r-value reference for moving

and both versions need to handle the possibility of self-assignment, manually implement the individual copies/moves, etc., or

The correct way for non-resource managing classes, follow the Rule of Zero: Every class should either manage an unmanaged resource (and nothing else), providing a copy constructor, assignment operator and destructor (in C 11, adding a move constructor and move assignment operator) or use solely managed resources and provide none of the above; the compiler will generate good ones for you.

Your original code was clearly going for option #2; your answer uses option #1's prototype, while still following option #2's template (involving a pointless self-assignment check), and on top of that, it does a crazy thing of not returning a reference to the object just assigned to (instead returning a temporary object that is never initialized to anything useful, and has nothing to do with any of the objects involved).

The minimalist solution here is to just fix your operator= for option #2 (since you've been told you should do a self-assignment check, and this is the only design that needs one):

Polynomial.cpp:

Polynomial& Polynomial::operator=(const Polynomial& op1)  {  // Changed to return by reference, and accept by const reference
    // self assignment guard
    if (this == &op1) {
        return *this;
    }
    // No need to declare Polynomial temp; the target of the assignment is *this, the source is op1, there's no reason for a third unrelated Polynomial
    this->coefficient = op1.coefficient;
    this->variable = op1.variable;
    this->exponent = op1.exponent;
    return *this;  // Returning *this by reference adheres to the protocol; you've just assigned, and the result of the expression is the object assigned to
}

Polynomial.h

Polynomial& operator=(const Polynomial&); // Same change as in .cpp, to return by ref, accept by const ref

That said, this is a silly solution. Your Polynomial contains nothing but std::string (and logically seems to have no reason for anything but variable to even be that; both coefficient and exponent seem like they should be an integer, floating point, or complex type, depending on use case), which is a managed type; adhering to the Rule of Zero would mean your class needn't implement copy assignment at all, nor copy construction, move construction, move assignment or a destructor, C would generate them all for you, simplifying your class to just:

Polynomial.h (now with nothing but the "from components" constructor and no assignment operators, because C makes them for you):

#pragma once
#ifndef POLYNOMIAL_H
#define POLYNOMIAL_H

#include <iostream>
#include <string>

class Polynomial {
    friend std::ostream& operator<<(std::ostream&, const Polynomial&);
    friend std::istream& operator>>(std::istream&, Polynomial&);

public:
    // Default Constructor

    Polynomial(std::string coefficient = "1", std::string variable = "x",
        std::string exponent = "4");

    void setPolynomial(std::string, std::string, std::string);
    Polynomial getPolynomial();
    // addition operator; Polynomial   Polynomial
    Polynomial operator (const Polynomial&) const;
    // subtraction operator; Polynomial - Polynomial
    Polynomial operator-(const Polynomial&) const;

private:
    std::string coefficient; // 
    std::string variable;    // 
    std::string exponent;    // 
};

#endif

Polynomial.cpp:


// Overloaded stream insertion and stream extraction operators
// for class PhoneNumber.
#include "Polynomial.h"
#include <utility>  // For std::move
#include <iomanip>
using namespace std;

// default constructor; conversion constructor that converts
// Changed: Added actual initialization of fields from arguments (previously unused)
Polynomial::Polynomial(std::string co, std::string va, std::string ex) : coefficient(std::move(co)), variable(std::move(va)), exponent(std::move(ex)) {}
// Setters
void Polynomial::setPolynomial(std::string co, std::string va, std::string ex) {
    // Changed: Using std::move since we received by value, so we can empty
    // large strings more efficiently by transferring ownership from the copies that
    // are about to go away anyway
    this->coefficient = std::move(co);
    this->variable = std::move(va);
    this->exponent = std::move(ex);
}
// Getters
// Note: This function makes no sense; it's basically equivalent to just using the
// copy constructor, so if you have Polynomial a and b:
// a = b.getPolynomial();
// is just a long-winded way to spell:
// a = b;
// (and it won't work if b is const, because the function wasn't declared const)
Polynomial Polynomial::getPolynomial() { return *this; }
// overloaded stream insertion operator; cannot be a member function
// if we would like to invoke it with cout << somePhoneNumber;
ostream& operator<<(ostream& output, const Polynomial& number) {
    output << "Coefficient: " << number.coefficient
        << "\nVariable: " << number.variable
        << "\nExponent: " << number.exponent << "\n"
        << "" << number.coefficient << "" << number.variable << "^"
        << number.exponent << "\n";
    return output; // enables cout << a << b << c;
}

// overloaded stream extraction operator; cannot be a member function
// if we would like to invoke it with cin >> somePhoneNumber;
istream& operator>>(istream& input, Polynomial& number) {
    input >> setw(1) >> number.coefficient; // input area code
    input >> setw(1) >> number.variable;    // input exchange
    input >> setw(1) >> number.exponent;    // input line
    return input;                           // enables cin >> a >> b >> c;
}

// addition operator; Polynomial   Polynomial
// A member function takes an implicit first parameter
Polynomial Polynomial::operator (const Polynomial& op1) const {
    Polynomial temp; // temporary result

    if (this->variable == op1.variable) {
        if (this->exponent == op1.exponent) {
            // Use stoi string to int
            int num1 = stoi(this->coefficient);
            int num2 = stoi(op1.coefficient);
            // use to_string to set coefficient
            // Changed: Assigning result of std::to_string to coefficient directly
            // avoiding temporary std::string s that would just add more copy/move work
            temp.coefficient = std::to_string(num1   num2);
            temp.variable = this->variable;
            temp.exponent = this->exponent;
        }
        else {
            std::cout << "Exponents must match\n";
        }
    }
    else {
        std::cout << "Variables must match\n";
    }
    return temp; // return copy of temporary object
}
// substraction operator; Polynomial - Polynomial
// A member function takes an implicit first parameter
Polynomial Polynomial::operator-(const Polynomial& op1) const {
    Polynomial temp; // temporary result

    if (this->variable == op1.variable) {
        if (this->exponent == op1.exponent) {
            // Use stoi string to int
            int num1 = stoi(this->coefficient);
            int num2 = stoi(op1.coefficient);
            // Changed: Assigning result of std::to_string to coefficient directly
            // avoiding temporary std::string s that would just add more copy/move work
            temp.coefficient = std::to_string(num1 - num2);
            temp.variable = this->variable;
            temp.exponent = this->exponent;
        }
        else {
            std::cout << "Exponents must match\n";
        }
    }
    else {
        std::cout << "Variables must match\n";
    }
    return temp; // return copy of temporary object
}

Try it online!

Additional notes:

As written, failed and - operations (due to mismatched variable/exponent) still behave as if they succeeded, returning a default initialized Polynomial. Sure, it dumps a log to std::cout, but the program still continues running with total garbage data. Those std::cout errors should really be exceptions, not simple logs.
Your comments claim the and - operations return a copy of temp. While logically this true (it must be copyable to be used in that context), in most C 11 and later compilers, copy elision will occur (the new Polynomial will be constructed directly into the caller allocated storage; no moves, let alone copies will occur).