I've created a class which can add two polynomial coefficients, which are simply the numbers in a dynamic array index positions. My main issue is with the overload+ function return value. Once I add two objects of the same class, the Visal Studio will give me an error if I try to return an object variable, but if I return the constructor to an object, then the code works fine, and there is no error. I don't know why I get this error, as in my previous assignments on dynamic arrays, I was returning object variables without any problems? I'm not sure what are the rules on return values for the operator+ overload function in this case? Thanks.
#include <iostream>
#include <vector>
using namespace std;
class polynomial
{
public:
polynomial();
polynomial(vector<double> vec);
polynomial(const polynomial& obj);
void get();
polynomial& operator=(const polynomial& rightSide);
friend const polynomial operator +(const polynomial& x, const polynomial& y);
//The function in question
~polynomial() { delete[] p; };
private:
double *p;
int expSize;
};
int main()
{
vector<double> x(3);
vector<double>y(3);
x = { 2,3,4 };
y = { 4,3,2 };
polynomial X(x); //<-- Both objects are constructed with vectors
polynomial Y(y);
polynomial obj;
obj = X + Y; //<-- Adding dynamic arrays, and saving the values in obj.
obj.get();
cout << endl;
system("pause");
return 0;
}
polynomial::polynomial()
{
expSize = 3;
p = new double[expSize];
for (int c = 0; c < expSize; c++)
p[c] = 0;
}
polynomial::polynomial(vector<double>vec)
{
expSize = vec.size();
p = new double[expSize];
for (int c = 0; c < expSize; c++)
p[c] = 0;
for (int c = 0; c < expSize; c++)
p[c] = vec[c];
}
polynomial::polynomial(const polynomial& obj)
{
p = new double[expSize];
for (int c = 0; c < expSize; c++)
p[c] = obj.p[c];
}
polynomial& polynomial::operator=(const polynomial& rightSide)
{
if (this == &rightSide)
return *this;
else
{
expSize = rightSide.expSize;
delete[] p;
p = new double[expSize];
for (int c = 0; c < expSize; c++)
p[c] = rightSide.p[c];
return *this;
}
}
const polynomial operator +(const polynomial& x, const polynomial& y)
{
polynomial obj;
if (x.expSize > y.expSize) //<-- obj.expSize private member variable will
obj.expSize = x.expSize; // inherit the larger dynamic array index size
else if(x.expSize <= y.expSize) // of the two parameter objects.
obj.expSize = y.expSize;
for (int c = 0; c < obj.expSize; c++) {
obj.p[c] = x.p[c] + y.p[c];
}
vector<double>vec(obj.expSize); //<-- Vector will inherit the new index size too.
for (int c = 0; c < obj.expSize; c++) {
vec[c] = obj.p[c]; //<-- Vector takes joined values of two objects
}
//return polynomial(vec); //<-- Returning a constructor with vector works fine
return obj; //<-- abort() has been called error
}
void polynomial::get()
{
for (int c = 0; c < expSize; c++)
cout << p[c] << endl;
}
3 Answers 3
I would just make some recommendations. copy ctor of polynomial has a bug. it should be:
polynomial::polynomial(const polynomial &obj)
{
expSize = obj.expSize;
p = new double[expSize];
for (int c = 0; c < expSize; c++) p[c] = obj.p[c];
}
overloaded vector ctor can be defined like this:
polynomial::polynomial(const vector<double> &vec)
{
expSize = vec.size();
p = new double[expSize];
for (int c = 0; c < expSize; c++) p[c] = vec[c];
}
assignment operator can be defined like this:
polynomial& polynomial::operator=(const polynomial &rightSide)
{
if (this != &rightSide)
{
//// if two polynomials have the same expSize then
//// they should have an array of the same size
if (expSize != rightSide.expSize)
{
expSize = rightSide.expSize;
delete[] p;
p = new double[expSize];
}
for (int c = 0; c < expSize; c++) p[c] = rightSide.p[c];
}
return *this;
}
addition operator does not need to be a friend; it can be:
class polynomial
{
polynomial operator+(const polynomial &rightSide) const
{
//
}
};
but if you must implement it as a friend:
polynomial operator+(const polynomial &x, const polynomial &y)
{
vector<double> vec;
for (int c = 0; ((c < x.expSize) || (c < y.expSize)); ++c)
{
vec.push_back(0);
if (c < x.expSize) vec.back() += x.p[c];
if (c < y.expSize) vec.back() += y.p[c];
}
//return polynomial(vec); OK
return vec; // also OK
}
Finally, I think you should implement a move ctor and a move assignment operator for your class.
5 Comments
return polynomial(vec); -- This could simply be return vec;Typical solution is to implement public operator+= as member of your class that
returns non-const reference to *this and then operator+ outside of class using it:
inline polynomial operator+(polynomial x, const polynomial& y) {
x += y;
return x;
}
or same with one line:
inline polynomial operator+(polynomial x, const polynomial& y) {
return x += y;
}
The compilers will try to elide as lot of copying or moving as these can in the process.
Note that it can be also written with parameter x being reference to const:
inline polynomial operator+(const polynomial& x, const polynomial& y) {
polynomial ret = x;
ret += y;
return ret;
}
Here we always copy x to ret. The other variant let caller to move into parameter x and so there were more opportunities of optimal usage.
2 Comments
To quickly answer your question: The proper way to return class instances from any function is to return any object supported by the constructor of that class.
NB: polynomials are best implemented with sparse arrays
The code defined in the following snippet is an improvement to your polynomial class. It is more efficient, it has a move ctor and also introduces the concept of a null array. Please study carefully.
#ifndef POLYNOMIAL_H
#define POLYNOMIAL_H
#include <iostream>
#include <initializer_list>
class polynomial
{
private:
int size; // size must always be > 0
double *array; // null value implies, array = [0];
public:
/*
1) default ctor
2) ctor providing support for initializer list : polynomial P = {1, 2, 3};
3) ctor providing support for STL containers like std::vector<> etc;
4) copy ctor
5) move ctor
*/
polynomial(void); // (1)
polynomial(const std::initializer_list<double>&); // (2)
template<template<typename...> class T, typename _Tp> polynomial(const T<_Tp>&); // (3)
polynomial(const polynomial&); // (4)
polynomial(polynomial&&); // (5)
~polynomial(void);
polynomial& operator=(const polynomial&);
polynomial& operator=(polynomial&&);
polynomial& operator=(const std::initializer_list<double>&);
template<template<typename...> class T, typename _Tp> polynomial& operator=(const T<_Tp>&);
int order(void) const;
void clear(void);
void set(const int &index, const double &value);
double get(const int &index) const;
double operator[](const int &index) const;
polynomial operator+(const polynomial&) const;
polynomial operator-(const polynomial&) const;
friend std::basic_ostream<char>& operator<<(std::basic_ostream<char> &O, const polynomial&);
};
#endif /* POLYNOMIAL_H */
here is an example that makes use of the polynomial class:
#include "polynomial.h"
using namespace std;
int main(int argc, char** argv)
{
polynomial A = {1, 2, 3};
polynomial B;
cout << "A: " << A.order() << endl;
cout << "B: " << B.order() << endl;
cout << (A + B) << endl;
return 0;
}
output is:
A: 2
B: 0
3(x^2) + 2x + 1
implementation of polynomial.cpp:
#include "polynomial.h"
#include <sstream>
#include <algorithm>
#include <list>
polynomial::polynomial(void) : size(1), array(nullptr)
{
//
}
polynomial::polynomial(const std::initializer_list<double> &args) : polynomial()
{
if (args.size() > 0) { size = args.size(); array = new double[size]; std::copy(args.begin(), args.end(), array); }
}
template<template<typename...> class T, typename _Tp> polynomial::polynomial(const T<_Tp> &data) : polynomial()
{
if (data.size() > 0) { size = data.size(); array = new double[size]; std::copy(data.begin(), data.end(), array); }
}
polynomial::polynomial(const polynomial &P) : polynomial()
{
if ((P.array != nullptr) && (P.size > 0)) { size = P.size; array = new double[size]; std::copy(&P.array[0], &P.array[size], array); }
}
polynomial::polynomial(polynomial &&P) : polynomial()
{
std::swap(size, P.size); std::swap(array, P.array);
}
polynomial::~polynomial(void)
{
if (array) { delete[] array; array = nullptr; } size = 0;
}
int polynomial::order(void) const
{
return (size - 1);
}
void polynomial::clear(void)
{
if (array) { delete[] array; array = nullptr; } size = 1;
}
polynomial& polynomial::operator=(const polynomial &P)
{
return ((this != &P) ? operator=(polynomial(P)) : (*this));
}
polynomial& polynomial::operator=(polynomial &&P)
{
if (this != &P) { clear(); std::swap(size, P.size); std::swap(array, P.array); } return (*this);
}
polynomial& polynomial::operator=(const std::initializer_list<double> &args)
{
return operator=(polynomial(args));
}
template<template<typename...> class T, typename _Tp> polynomial& polynomial::operator=(const T<_Tp> &data)
{
return operator=(polynomial(data));
}
void polynomial::set(const int &i, const double &d)
{
if ((size < 1) || (i < 0) || (i >= size)) throw "index out of bounds!";
if (array == nullptr) { array = new double[size]; } array[i] = d;
}
double polynomial::get(const int &i) const
{
if ((size < 1) || (i < 0) || (i >= size)) throw "index out of bounds!";
return ((array == nullptr) ? 0 : array[i]);
}
double polynomial::operator[](const int &i) const
{
return get(i);
}
polynomial polynomial::operator+(const polynomial &B) const
{
std::list<double> data;
auto &A = (*this);
for (int i = 0; ((i < A.size) || (i < B.size)); ++i)
{
data.push_back(0);
if (i < A.size) data.back() += A.get(i);
if (i < B.size) data.back() += B.get(i);
}
return data;
}
polynomial polynomial::operator-(const polynomial &B) const
{
std::list<double> data;
auto &A = (*this);
for (int i = 0; ((i < A.size) || (i < B.size)); ++i)
{
data.push_back(0);
if (i < A.size) data.back() += A.get(i);
if (i < B.size) data.back() -= B.get(i);
}
return data;
}
std::basic_ostream<char>& operator<<(std::basic_ostream<char> &O, const polynomial &P)
{
if ((P.array != nullptr) && (P.size > 0))
{
std::basic_stringstream<char> sout;
sout.precision(O.precision());
sout.flags(sout.flags() | (O.flags() & std::ios::floatfield));
for (int i = (P.size - 1), j = 0; (i >= 0); --i, ++j)
{
if (j == 0) sout << P.array[i];
else sout << ((P.array[i] < 0) ? " - " : " + ") << std::abs(P.array[i]);
if (i >= 2) sout << "(x^" << i << ')';
if (i == 1) sout << 'x';
}
O << sout.str();
}
else
{
O << 0;
}
return O;
}
I hope this is helpful
operator =has flaws, and can be written much more easily:polynomial& polynomial::operator=(const polynomial& rightSide) { if ( this != &rightSide) { polynomial temp(rightside); std::swap(temp.expSize, expSize); std::swap(temp.p, p); } return *this; }. This uses the copy/swap idiompolynomial objcreate an array of 3 items (see default constructor code) and you operator + does not take this into account so you will have undefined behavior in your program.