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
}
Additional notes:
As written, failed
-
operations (due to mismatched variable/exponent) still behave as if they succeeded, returning a default initializedPolynomial
. Sure, it dumps a log tostd::cout
, but the program still continues running with total garbage data. Thosestd::cout
errors should really be exceptions, not simple logs.Your comments claim the
-
operations return a copy oftemp
. 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 newPolynomial
will be constructed directly into the caller allocated storage; no moves, let alone copies will occur).