2

When creating a class like this one:

class Test {
 public:
   ...

private:
   string s1_;
   string s2_;
   vector<int> v_;
};

What is the best way to declare a constructor accepting two strings and a vector? And, more specifically, how do you handle lvalue and rvalue references?

I see the three following options:

  1. Create every combination of lvref and rvref:

       Test(const string& s1, const string& s2, const vector<int>& v) :
          s1_{s1}, s2_{s2}, v_{v}
       {
          ...
       }
    
       Test(const string& s1, const string& s2, vector<int>&& v) :
          s1_{s1}, s2_{s2}, v_{move(v)}
       {
          ...
       }
    
       Test(const string& s1, string&& s2, const vector<int>& v) :
          s1_{s1}, s2_{move(s2)}, v_{v}
       {
          ...
       }
    
       Test(const string& s1, string&& s2, vector<int>&& v) :
          s1_{s1}, s2_{move(s2)}, v_{move(v)}
       {
          ...
       }
    
       Test(string&& s1, const string& s2, const vector<int>& v) :
          s1_{move(s1)}, s2_{s2}, v_{v}
       {
          ...
       }
    
       Test(string&& s1, const string& s2, vector<int>&& v) :
          s1_{move(s1)}, s2_{s2}, v_{move(v)}
       {
          ...
       }
    
       Test(string&& s1, string&& s2, const vector<int>& v) :
          s1_{move(s1)}, s2_{move(s2)}, v_{v}
       {
          ...
       }
    
       Test(string&& s1, string&& s2, vector<int>&& v) :
          s1_{move(s1)}, s2_{move(s2)}, v_{move(v)}
       {
          ...
       }
    

    Pros: Every possibility handled efficiently.

    Cons: Requires a lot of code to handle every combination and might be error prone.

  2. Always copy and move the arguments:

       Test(string s1, string s2, vector<int> v) :
          s1_{move(s1)}, s2_{move(s2)}, v_{move(v)}
       {
          ...
       }
    

    Pros: Only one ctor.

    Cons: Not as efficient because move does not mean free.

  3. Use "universal references":

       template <typename S1, typename S2, typename V>
       Test(S1&& s1, S2&& s2, V&& v) :
          s1_{forward<S1>(s1)}, s2_{forward<S2>(s2)}, v_{forward<V>(v)}
       {
          ...
       }
    

    Pros: One ctor that handles everything efficiently.

    Cons: Not really meaningful. What are s1, s2 and v? Can be even more error prone (e.g. Test error{1,2,3} compiles).

Is there a better way to achieve that?

ildjarn
  • 62,044
  • 9
  • 127
  • 211
slepasteur
  • 186
  • 1
  • 11
  • 1
    Note: They're not called "left value references". They're just "lvalue references". It's not an abbreviation or anything; the proper term is "lvalue". – Nicol Bolas Apr 16 '13 at 14:18
  • 1
    _Use universal references_ - universal reference is `S1&& s`, but not `S1 s`. – awesoon Apr 16 '13 at 14:18
  • 4
    "Move does not mean free", but until you've identified a performance issue, it might as well be. Remember that the compiler can optimise out primitive copies on the stack. – ecatmur Apr 16 '13 at 14:25
  • @soon Yes of course S1&&, sorry for the typo, I edited to correct this. However to be sure that a universal reference ctor is used correctly you have to rely on std::enable_if. It's not really easy to understand at first what is intended. – slepasteur Apr 16 '13 at 19:56
  • @Nicol Bolas My bad, I always assumed that lvalue meant left value. Thank you for correcting me. – slepasteur Apr 16 '13 at 19:58
  • @piemur BTW, the name `lvalue` historically *does* originally come from the concept "left value", but the proper name from the standard and in common technical use really is `lvalue`. If you try to expand it back out to "left value" when talking about an `lvalue` it is just confusing and non-specific. – wjl Feb 23 '14 at 02:28

1 Answers1

0

What about:

template <typename String, typename VectorInt>
Test(String &&s1, String &&s2, VectorInt &&v,
  typename std::enable_if<std::is_same<typename std::decay<String>::type,std::string>::value &&
  std::is_same<typename std::decay<VectorInt>::type,std::vector<int>>::value>::type * = nullptr) :
  s1_(std::forward<String>(s1)), s2_(std::forward<String>(s2)),
  v_(std::forward<VectorInt>(v))
{}
bluescarni
  • 3,937
  • 1
  • 22
  • 33