Implementing Scalar and Vector Addition for Custom Type with std::transform

Question

This looks to me like a basic problem so apologies in advance for duplicate posts. Not sure what the terminology is though.

I have a class, say my_data, for storing numbers. It has the basic constructors etc. and it would be handy to support elementary operations such as adding to the elements, e.g. increasing the values in the data.

This is the most elementary way to describe the class:

#include <stdlib.h>
#include <vector>
#include <iostream>     
#include <algorithm>     
#include <numeric>      
#include <iterator>     

template <typename T>

class my_data {

public:
  my_data  (const my_data& other);
  my_data& operator=(const my_data& rhs);
  my_data  (const size_t bsize) { my_datavec.resize(bsize); }
  my_data  (std::vector<T> init) { my_datavec = init; }  

  template <typename U>
  void add(const U addition) {
    std::transform(my_datavec.begin(), my_datavec.end(), 
                   my_datavec.begin(), 
                   bind2nd(std::plus<T>(), addition) );
  }

  template <typename U>
  void add(const my_data<U> addition) {
     std::transform(my_datavec.begin(), my_datavec.end(), 
                    addition.my_datavec.begin(),
                    my_datavec.begin(), 
                    std::plus<T>() );
  }  

  void print() {    
    std::ostream_iterator<T> out_it (std::cout,", ");
    std::copy(my_datavec.begin(), my_datavec.end(), out_it);
  }

protected:
  std::vector<T> my_datavec;

};

int main() {

  std::vector<int> int_test({1,2,3});
  my_data<int> a(int_test);

  a.add(2);
  a.print();

  a.add(a);
  a.print();

  return(0);

}

The problem starts when I want to add the values of one object to the values of another (abusing the fact here that they are the same size to omit checks):

$ g++ -std=c++11 template2.cpp -o template2
In file included from /usr/include/c++/4.7/bits/stl_function.h:741:0,
             from /usr/include/c++/4.7/string:50,
             from /usr/include/c++/4.7/bits/locale_classes.h:42,
             from /usr/include/c++/4.7/bits/ios_base.h:43,
             from /usr/include/c++/4.7/ios:43,
             from /usr/include/c++/4.7/ostream:40,
             from /usr/include/c++/4.7/iostream:40,
             from template2.cpp:3:
/usr/include/c++/4.7/backward/binders.h: In instantiation of ‘std::binder2nd<_Operation> std::bind2nd(const _Operation&, const _Tp&) [with _Operation = std::plus<int>; _Tp = my_data<int>]’:
template2.cpp:20:5:   required from ‘void my_data<T>::add(U) [with U = my_data<int>; T = int]’
template2.cpp:45:10:   required from here
/usr/include/c++/4.7/backward/binders.h:170:57: error: invalid cast from type ‘const my_data<int>’ to type ‘_Arg2_type {aka int}’

The code of adding two my_data objects together does not seem to be used. Of course <typename U> can be objects of type X as well as my_data<X> but how do I let the compiler know when to use the second version of add()?

My current version of the program uses

void add (const bisArray<U> addition, const std::true_type&) 

for adding an element and

void add (const bisArray<U> addition, const std::false_type&) 

for adding a my_data object (the calls are a.add(2, std::is_arithmetic<int>::type() ); for adding an element and a.add(a, std::is_arithmetic<my_data<int>>::type() ); for adding a my_data object).

It is not really a solution though because there is nothing to prevent the occurrence of the calls a.add( a , std::is_arithmetic<int>::type() ); and a.add( 2 , std::is_arithmetic<my_data<int>>::type() ); which lead to a segmentation fault.

Is there a mechanism to resolve this more elegantly?

Answer 1

There are a number of issues with your code. The most immediate reason for your code not compiling was already pointed out by Igor Tandetnik in the comments.

I'll start in “declaration order” which is not the order of severity:

1. The correct header to #include for std::size_t is not stdlib.h but cstddef. (Generally, don't #include the *.h but the c* versions.)
2. You probably want to add a default constructor that initializes your type with an empty vector.
3. You probably want to make the constructors that take one argumrnt explicit. It avoids bad surprises when an integer suddenly converts to a my_data.
4. The constructor that takes a std::vector<T> needlessly makes a copy of its argument. At the very least, you should std::move it into its destination. You might also want to consider providing an overload for const std::vector<T>& (which does a copy) and std::vector<T>&& (which does a move).
5. Prefer initializer lists over assignment in the constructor.
6. Generally prefer C++11's uniform initialization syntax ({ … }).
7. Use C++11 lambdas instead of the legacy binders. It's more readable and at least as efficient.
8. Consider overloading operators += and << instead of providing named member functions.
9. Your vector addition needs a size-check. std::transform cannot tell how long the second and third ranges are.
10. The addition functions shouldn't make a needless copy of their argument.
11. In the vector addition's std::transform call, a binary function should be provided. Since you have templated the function, neither std::plus<T> nor std::plus<U> are applicable. Use a lambda that adds a T and a U. This will also be more efficient if, for example T = std::string and U = char *.

Let's put all this together:

#include <algorithm>
#include <cstddef>
#include <iostream>
#include <iterator>
#include <stdexcept>
#include <vector>

template<typename T>
class my_data
{

public:

  my_data()
  {
  }

  explicit my_data (const std::size_t size)
  {
    data.resize(size);
  }

  explicit my_data(const std::vector<T>& init) : data {init}
  {
  }

  explicit my_data(std::vector<T>&& init) : data {std::move(init)}
  {
  }

  template<typename U>
  void
  operator+=(const U& a)
  {
    std::transform(data.begin(),
                   data.end(),
                   data.begin(),
                   [&a](const T& b){ return a + b; });
  }

  template<typename U>
  void
  operator+=(const my_data<U>& other)
  {
    if (other.data.size() != this->data.size())
      throw std::invalid_argument {"incompatible sizes"};
    std::transform(data.begin(),
                   data.end(),
                   other.data.begin(),
                   data.begin(),
                   [](const T& a, const U& b){ return a + b; });
  }

  friend
  std::ostream&
  operator<<(std::ostream& os, const my_data& md)
  {
    std::ostream_iterator<T> outit {os, ", "};
    std::copy(md.data.begin(), md.data.end(), outit);
    return os;
  }

protected:

  std::vector<T> data {};
};

int main() {
  std::vector<int> inttest {1, 2, 3};
  my_data<int> a {inttest};
  std::cout << a << std::endl;
  a += 2;
  std::cout << a << std::endl;
  a += a;
  std::cout << a << std::endl;
}

Output:

1, 2, 3, 
3, 4, 5, 
6, 8, 10, 

One more thing that you might wish to address is the superfluous trailing comma in the output.