HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppModerate

Calculator using class templates

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
templatesclasscalculatorusing

Problem

I've just completed an assignment about class templates in C++. It works fine and produces the correct output. We were given the main function and had to construct and implement the class. I'm wondering if there is a better (more efficient) way to construct and implement this simple calculator class template.

#include 
#include 

using std::cout;
using std::endl;
using std::string;

template 
class Calculator{
public:
      Calculator(T1 first, T2 second);    
      Calculator( const Calculator& otherCalculator);      
      void setValue1(T1 first);
      void setValue2(T2 second);
      T1 add();
      T1 multiply();
 private:
      T1 number1;
      T2 number2;
 };

 template 
 Calculator::Calculator(T1 first, T2 second)
      :number1(first), number2(second)
 {}

 template 
 Calculator::Calculator(const Calculator& otherCalculator)
      :number1(otherCalculator.number1), number2(otherCalculator.number2)
 {}

 template 
 void Calculator::setValue1(T1 first){
      number1 = first;
 }

 template 
 void Calculator::setValue2(T2 second){
      number2 = second;
 }

 template 
 T1 Calculator::add(){
      T1 answer = number1 + number2;
      return answer;
 }

template 
T1 Calculator::multiply(){
      T1 answer = number1 * number2;
      return answer;
}

int main(){
      Calculator simpleCalc(42, 3);
      cout  copyCalc( simpleCalc);
      cout  realCalc(1.41421, 2.718281828459045);
      cout << "Adding: " << realCalc.add() << endl;
      cout << "Multiplying: " << realCalc.multiply() << endl;
      cout << endl;
      return 0;
}

Solution

-
Don't flush where you don't need to.

Flushing is expensive, so don't use std::endl unless you need it.

Nearly always, the shorter '\n' suffices, which can be merged with any bordering string-literal.

Still, that advice seems to be for your teacher, not you ;-)

-
return 0; is implicit in main if control reaches the closing brace }.

The same is true for C since C99.

This also seems to be one for your teacher.

-
I see you corrected your formatting since posting on SO.

Still, 6 spaces is unusual. Either use the standard tab-stop (8), or if that's too much for your tastes, 4 is a common choice.

Automatic formatting-tools exist which will do this and much additional formatting for you, which gain importance with the size of the project, especially the number of contributors.

-
There is no reason to write the member-functions of the template out-of-line.

Even if you later divide your projects in multiple files, and thus have headers and implementation-files, templates are always (with really few exceptions) implemented in the header anyway, due to technical constraints.

-
There is absolutely no reason to explicitly implement the copy-constructor.

Let the compiler provide a default one.

Since C++11, one can also explicitly default it, preferably at the point of declaration: = default;.

Additional benefit: Doing so re-enables move-ctor and move-assignment.

-
The 2-argument-ctor should be templated to accept any assignable types and use perfect forwarding:

template ()),
    T2(std::declval()), void())>
Calculator(T3&& a, T4&& b) : number1(std::forward(a)), number2(std::forward(b)) {}


-
The setters should accept any assignable type and use perfect forwarding for optimal performance.

template 
auto setValue1(T3&& x) -> decltype(number1 = std::forward(x), void())
{ number1 = std::forward(x); }
template 
auto setValue2(T3&& x) -> decltype(number2 = std::forward(x), void());
{ number2 = std::forward(x); }


-
add and multiply should use a deduced return-type, and be const.
This transfers to avoiding explicit types in the implementations:

-
Use decltype(std::declval()+std::declval()) for C++11.

decltype(std::declval()+std::declval()) add() const;
decltype(std::declval()*std::declval()) multiply() const;


-
Or since C++14, just auto for letting the compiler deduce it.

auto add() const {
    auto answer = number1 + number2;
    return answer;
}


-
Or shorter:

auto add() const {
    return number1 + number2;
}


Putting it all together:

template 
class Calculator {
    T1 number1;
    T2 number2;
public:
    template ()),
        T2(std::declval()), void())>
    Calculator(T3&& a, T4&& b) : number1(std::forward(a)), number2(std::forward(b)) {}
    // implicit: copy-ctor, move-ctor, copy-assignment, move-assignment, dtor

    template 
    auto setValue1(T3&& x) -> decltype(number1 = std::forward(x), void())
    { number1 = std::forward(x); }
    template 
    void setValue2(T3&& x) -> decltype(number2 = std::forward(x), void());
    { number2 = std::forward(x); }

    auto add() const { return number1 + number2; }
    auto multiply() const { return number1 * number2; }
};

Code Snippets

template <typename T3, typename T4, typename = decltype(T1(std::declval<T3&&>()),
    T2(std::declval<T4&&>()), void())>
Calculator(T3&& a, T4&& b) : number1(std::forward<T3>(a)), number2(std::forward<T4>(b)) {}
template <typename T3>
auto setValue1(T3&& x) -> decltype(number1 = std::forward<T3>(x), void())
{ number1 = std::forward<T3>(x); }
template <typename T3>
auto setValue2(T3&& x) -> decltype(number2 = std::forward<T3>(x), void());
{ number2 = std::forward<T3>(x); }
decltype(std::declval<T1&>()+std::declval<T2&>()) add() const;
decltype(std::declval<T1&>()*std::declval<T2&>()) multiply() const;
auto add() const {
    auto answer = number1 + number2;
    return answer;
}
auto add() const {
    return number1 + number2;
}

Context

StackExchange Code Review Q#79768, answer score: 15

Revisions (0)

No revisions yet.