How can I create a stack w/arithmetic operations

Question

Microsoft Visual Studio Community 2019
Version 16.11.2
Visual Studio.16.Release/16.11.2+31624.102
Installed Version: Community
Visual C++ 2019   00435-60000-00000-AA535
Microsoft Visual C++ 2019

I'm trying to create a simple stack and include simple arithmetic operations. The containing class is templated. I thought that the arithmetic operations of the class template would be applied and allow the arithmetic operations to proceed.

The error messages with the following code are:

Error   C2679   binary '+=': no operator found which takes a right-hand operand of type 'Values' (or there is no acceptable conversion) line 194    
Error   C2679   binary '+=': no operator found which takes a right-hand operand of type 'Values' (or there is no acceptable conversion) line 196    
Error   C2679   binary '+=': no operator found which takes a right-hand operand of type 'Values *' (or there is no acceptable conversion) line195

---- code ----

# include <stack>
# include <deque>

using namespace std;

class Values {
   long double value[3] = {0.0, 0.0, 0.0};
public:
   Values() {}
   Values(Values& v) { asgn(v); };
   Values(long double value0, long double value1, long double value2) {
      asgn(value0, value1, value2);
   };
   long double& operator[](const int i) { return value[min(2, max(0, i))]; };
   Values& operator+=(Values  value) { return sumt(value);   };
   Values& operator+=(Values* value) { return sumt(*value);  };
   Values& operator+=(long double v) { return sumt(v);       };
   Values& operator-=(Values  value) { return subt(value);  };
   Values& operator-=(Values* value) { return subt(*value); };
   Values& operator-=(long double v) { return subt(v);      };
   Values& operator*=(Values  value) { return mult(value);  };
   Values& operator*=(Values* value) { return mult(*value); };
   Values& operator*=(long double v) { return mult(v);      };
   Values& operator/=(Values  value) { return divt(value);  };
   Values& operator/=(Values* value) { return divt(*value); };
   Values& operator/=(long double v) { return divt(v);      };
   Values& operator=( Values  value) { return asgn(value);  };
   Values& operator=( Values* value) { return asgn(*value); };
   Values& operator=( long double v) { return asgn(v);      };
   Values& operator+( Values  value) { return add(value);   };
   Values& operator+( long double v) { return add(v);       };
   Values& operator-( Values  value) { return sub(value);   };
   Values& operator-( long double v) { return sub(v);       };
   Values& operator*( Values  value) { return mul(value);   };
   Values& operator*( long double v) { return mul(v);       };
   Values& operator/( Values  value) { return div(value);   };
   Values& operator/( long double v) { return div(v);       };
   Values& asgn(Values& value) {
      this->value[0] = value[0];
      this->value[1] = value[1];
      this->value[2] = value[2];
      return *this;
   };
   Values& asgn(long double v) {
      value[0] = value[1] = value[2] = v;
      return *this;
   };
   Values& asgn(const long double value0, const long double value1, const long double value2) {
      value[0] = value0;
      value[1] = value1;
      value[2] = value2;
      return *this;
   };
   Values& sumt(Values& value) {
      this->value[0] += value[0];
      this->value[1] += value[1];
      this->value[2] += value[2];
      return *this;
   };
   Values& sumt(long double v) {
      this->value[0] += v;
      this->value[1] += v;
      this->value[2] += v;
      return *this;
   };
   Values& subt(Values& value) {
      this->value[0] -= value[0];
      this->value[1] -= value[1];
      this->value[2] -= value[2];
      return *this;
   };
   Values& subt(long double v) {
      this->value[0] -= v;
      this->value[1] -= v;
      this->value[2] -= v;
      return *this;
   };
   Values& mult(Values& value) {
      this->value[0] *= value[0];
      this->value[1] *= value[1];
      this->value[2] *= value[2];
      return *this;
   };
   Values& mult(long double v) {
      this->value[0] *= v;
      this->value[1] *= v;
      this->value[2] *= v;
      return *this;
   };
   Values& divt(Values& value) {
      this->value[0] /= value[0];
      this->value[1] /= value[1];
      this->value[2] /= value[2];
      return *this;
   };
   Values& divt(long double v) {
      this->value[0] /= v;
      this->value[1] /= v;
      this->value[2] /= v;
      return *this;
   };
   Values& add(Values& value) {
      Values v;
      v[0] = this->value[0] + value[0];
      v[1] = this->value[1] + value[1];
      v[2] = this->value[2] + value[2];
      return v;
   };
   Values& add(long double v) {
      Values value;
      value[0] = this->value[0] + v;
      value[1] = this->value[1] + v;
      value[2] = this->value[2] + v;
      return value;
   };
   Values& sub(Values& value) {
      Values v;
      v[0] = this->value[0] - value[0];
      v[1] = this->value[1] - value[1];
      v[2] = this->value[2] - value[2];
      return v;
   };
   Values& sub(long double v) {
      Values value;
      value[0] = this->value[0] - v;
      value[1] = this->value[1] - v;
      value[2] = this->value[2] - v;
      return value;
   };
   Values& mul(Values& value) {
      Values v;
      v[0] = this->value[0] * value[0];
      v[1] = this->value[1] * value[1];
      v[2] = this->value[2] * value[2];
      return v;
   };
   Values& mul(long double v) {
      Values value;
      value[0] = this->value[0] * v;
      value[1] = this->value[1] * v;
      value[2] = this->value[2] * v;
      return value;
   };
   Values& div(Values& value) {
      Values v;
      v[0] = this->value[0] / value[0];
      v[1] = this->value[1] / value[1];
      v[2] = this->value[2] / value[2];
      return v;
   };
   Values& div(long double v) {
      Values value;
      value[0] = this->value[0] / v;
      value[1] = this->value[1] / v;
      value[2] = this->value[2] / v;
      return value;
   };
}; // class Values

template <class T>
class Stack {
   stack<T> s;
public:
   Stack() {}
   Stack(const Stack& value) { for (T v : value) s.push(v); }
   ~Stack() { }
   T&     push(T item)        { s.push(item); return item; }
   T      pop()               { T v = s.top(); s.pop(); return v; }
   T&     top()               { return s.top();   }
   bool   empty()             { return s.empty(); }
   size_t size() const        { return s.size();  }
   T& operator+=(T& other)    {s.top() += other; return s.top(); }
   T& operator-=(T& other)    {s.top() -= other; return s.top(); }
   T& operator*=(T& other)    {s.top() *= other; return s.top(); }
   T& operator/=(T& other)    {s.top() /= other; return s.top(); }
   T& operator+ (T& other)    { return (s.top() + other); }
   T& operator- (T& other)    { return (s.top() - other); }
   T& operator* (T& other)    { return (s.top() * other); }
   T& operator/ (T& other)    { return (s.top() / other); }
   T& add()                   { T item = s.top(); s.pop(); return (s.top() += item); }
   T& sub()                   { T item = s.top(); s.pop(); return (s.top() -= item); }
   T& mul()                   { T item = s.top(); s.pop(); return (s.top() *= item); }
   T& div()                   { T item = s.top(); s.pop(); return (s.top() /= item); }
};

int main(int argc, char** argv) {
   Stack<Values> s;
   Values  v = Values(1.0, 2.0, 3.0);
   Values* z = new Values(1.0, 2.0, 3.0);
   s.push(v);
   Values x = s.top();

   s += Values(1.0, 2.0, 3.0);
   s += new Values(1.0, 2.0, 3.0);
   s.top() += Values(1.0, 2.0, 3.0);

   s +=  v;
   s += *z;
   s.top() += new Values(1.0, 2.0, 3.0);
   x += new Values(1.0, 2.0, 3.0);
   x += 2.0;
   return 0;
}

Whats the meaning of " Seems that I can't do this with my code" ? Do you get compiler errors? Some test cases that fail? Please explain what is wrong with the code — 463035818_is_not_an_ai, Sep 17 '21 at 08:46
Please take some time to refresh [the help pages](http://stackoverflow.com/help), take the SO [tour], read [ask], as well as [this question checklist](https://codeblog.jonskeet.uk/2012/11/24/stack-overflow-question-checklist/). Lastly please don't forget how to create a [mre]. — Some programmer dude, Sep 17 '21 at 08:51
The parameters and return types of your functions are somewhat strange. the result of `push` dangles, and you don't use the `item` parameter of `pop`. — Caleth, Sep 17 '21 at 09:14
Thanks for pointing my 'pop' error. You are indeed correct. The code above eliminates the argument. Could you explain what is strange about the return types so that I can fix them? I don't understand what you mean when you say "the result of push dangles". Could you explain this? — lostbits, Sep 17 '21 at 16:02
I think it's time for you to learn about [value categories](https://en.cppreference.com/w/cpp/language/value_category). For example, `Values(1.0, 2.0, 3.0)` is a prvalue and as such can't be bound to non-constant references. So for example a function taking a `Values&` argument can't be used with such a value, you need either rvalue refernces (ì.e. `Values&&`), constant lvalue references (i.e. `Values const&`), or plain non-reference values (i.e. `Values`). Now think about that in the context of your operator overloads. — Some programmer dude, Sep 17 '21 at 20:45
And to be honest, perhaps brutally so, any [decent C++ book](https://stackoverflow.com/a/388282/440558), tutorial or class should have said something about this. — Some programmer dude, Sep 17 '21 at 20:46
The above code is far from minimal. Remove all operators that are nor relevant to the question. Also clearly indicate which ine has the problem. Do you really think that people will count up to 196in the code hoping that line still match your original error? By the way there are a lot of issues in your code like missing const, memory leaks... You really don't want the overload taking a pointer. Also you sometime return reference to object on the stack which is **undefined behavior**. Better to learn the langage a bit more before writting such class by **reading 3 or 4 good books**, — Phil1970, Sep 18 '21 at 15:46

score 0 · Answer 1 · answered Sep 18 '21 at 16:32

There are many, many issues with your code.

Honestly, you should read a few good C++ books before writing such class as we can easily see that you don't properly understand a lot of C++ basic concept that everyone should know.

Namespace

You should avoid using namespace std at global scope as it is a bad practice.

long double

As you are using Visual C++, long double is the same size as double. With other compiler, it might make the code slower. Unless, you need the range, (in which case you would use another compiler), I think you should use double

Copy constructor

Your copy constructor should always receive a constant reference. And most of the time, it should not delegate to another function as it should be the basic one. Thus it should be written like this:

    Values(const Values &v)
    {
        std::copy(std::begin(v.value), std::end(v.value), std::begin(value));
    }

However, this is useless as the default implementation works and you can simple write instead:

    Value(const Value &) = default;

Operator[]

You need a constant version for sure. Only, if you want to be able to modify data, you should provide the non const one too.

Return a value, not a reference. Could also return a const reference too.

    long double operator[](const int i) const
    {
        return value[min(2, max(0, i))];
    };

Define this one only if you want write access to data.

    long double &operator[](const int i)
    { 
        return value[min(2, max(0, i))]; 
    };

Operator += and those with similar code

The first one should receive a const reference. Usually, it would the the basic operation on which other rely (not shown here)

    Values &operator+=(const Values  &value) 
    { 
        this->value[0] += value[0];
        this->value[1] += value[1];
        this->value[2] += value[2];
        return *this;
    };

The one that receive a pointer should not be provided. But if it is, then it should be a pointer to const data too.

    Values &operator+=(const Values *value) 
    { 
        return *this += *value; 
    };

Here also, it would be a primitive operation...

    Values &operator+=(long double v) 
    { 
        this->value[0] += v;
        this->value[1] += v;
        this->value[2] += v;
        return *this;
    };

Never return a reference (or pointer) to dangling data

Here you are returning a rederence to local dat which goes out of scope as soon as you exit the function. It is undefined behavior and the most probable output would be to get garbage.

    Values &add(Values &value) {
        Values v;
        v[0] = this->value[0] + value[0];
        v[1] = this->value[1] + value[1];
        v[2] = this->value[2] + value[2];
        return v;
    };

Also, not very efficient to do it that way as you define generally efficient function using inefficient one instead

You should return by value in that case. And here would be a typical implementation. That function is somewhat useless. However, similar code could be used for operator +

    Values add(const Values &value)
    {
        auto temp(*this);
        temp += value;
        return temp;
    }

Other missing const

    bool empty() const { return s.empty(); }

    T &operator+=(const T &other) { s.top() += other; return s.top(); }

Memory leaks

In the following code, there are 3 memory leaks (one for each new)

    Stack<Values> s;
    Values *z = new Values(1.0, 2.0, 3.0);
    s += new Values(1.0, 2.0, 3.0);
    s += *z;
    s.top() += new Values(1.0, 2.0, 3.0);
    x += new Values(1.0, 2.0, 3.0);

Other fixes

Well, I only show main problems and only once for each type... Do any other required adjusment.

Thanks for the time spent on answering me. I would take some exception to your statement on my qualifications. Visual Studio has been challenging. I currently have 15 accepted bug reports on their C++ compilers, and the bugs have forced changes to my code. some examples: default constructors: Vallues(const Values& v) {values[0] = v[0];} produces an error on '['. Values(const Values&v) = default; is not accepted. operator?(Values&) is in error. Error line number are wrong, in one case by 1000 lines. Few errors point out the correct error. And so on. What you see is a compromise -MSVS code. — lostbits, Sep 20 '21 at 14:38
One more thing. Some of your remarks on coding quality and suggested changes have already been tried and been flagged as an error. It has been frustrating to not produce code of any good quality. However, some of your comments have been successfully applied, and some will not be applied for various reasons. But what I leave you with is the notion that given a better compiler, you would have seen better code. — lostbits, Sep 20 '21 at 14:44
Almost all of the above suggestions have been tested and **do works** with Visual Studio even in C++ 14 mode. **Stop blaming the compiler**. The reason you get an error on `Vallues(const Values& v)` constructor is a **mispelling** as you have 2 l in Vallues. **Defaulted copy constructor** clearly works in the above case. — Phil1970, Sep 21 '21 at 01:38

How can I create a stack w/arithmetic operations

1 Answers1