-1

My question was, how the displayed behaviour could be prevented, but as Cheersandhth.-Alf pointed out, I failed to include my constructor which caused this problem. The constructor is now included and it is obvious, that forwarding the float (aswell as anything else) to the std::array ctor caused this problem.

I would still like to be able to use this kind of initialization Vec2 a = {1.f, 2.f}, but the forwarding ctor is pretty dangerous, so I will avoid that.

I have a Vec class derived from std::array which is supposed to implement the usual component-wise arithmetic operations via operator overloads. The operators should be implemented both for other Vecs of the same type and size (in that case operating on the corresponding vector components), aswell as integral and floating point types.

e.g.

{1.f, 2.f, 3.f} * 2.f = {2.f, 4.f, 6.f}
{1.f, 0.f} + {0.f, 1.f} = {1.f, 1.f}

Here is what I did (only shown for operator*) https://godbolt.org/g/PtCkzR:

template<class T, size_t N>
class Vec : public std::array<T, N>
{
public:
  template<typename... S>
  Vec(S&&... params) : std::array<T, N>{std::forward<S>(params)...} { };

  friend Vec<T, N>& operator*=(Vec<T, N>& a, const Vec<T, N>& b)
  {
    std::transform(a.begin(), a.end(), b.begin(), a.begin(), std::multiplies<>());
    return a;
  }

  template<class S>
  friend Vec<T, N>& operator*=(Vec<T, N>& a, const S& b)
  {
    std::transform(a.begin(), a.end(), a.begin(), [&] (T x) { return x*b; });
    return a;
  }

  template<class S>
  friend Vec<T, N> operator*(Vec<T, N> a, const S& b)
  {
    return a *= b;
  }
};
using Vec2 = Vec<float, 2>;

Now, when I want to multiply a vector with a float this happens:

Vec2 a{1.f, 1.f};
auto b = a * 0.5f; // b = {.5f, .5f} <- as expected
auto c = 0.5f * a; // c = {.5f, 0.f} <- what happened here?

This happens because the 0.5f in the third line is implicitly converted to a Vec2 {0.5f, 0.f} and is then passed to the operator*(Vec2, const Vec2&) overload.

eike
  • 1,314
  • 7
  • 18
  • 2
    You have `friend Vec operator*(Vec a, const S& b)`, also write `friend Vec operator*(const S& b, Vec a)` and that should fix your problem. – Justin Feb 14 '18 at 19:58
  • You should make multiplication operator non-template, `operator*(Vec a, const T& b)` and provide variant taking a scalar as a first argument, `operator*(const T& b, Vec a)`. – user7860670 Feb 14 '18 at 19:59
  • 2
    OT: I strongly recommend you don't inherit from `std::array`, but instead use it as a member variable. Prefer *Composition over Inheritance* – Justin Feb 14 '18 at 19:59
  • "Do I really have to implement all of the templated operator functions once for Vec2 op S and once for S op Vec2?" Is what I asked at the bottom. So there is no other way? I thought implementing these operators as friend functions or outside the class were ways of avoiding that extra code. – eike Feb 14 '18 at 20:00
  • @eike Yes, you do. – Justin Feb 14 '18 at 20:01
  • @Justin Would you mind elaborating on the suggestion to not inherit from std::array? – eike Feb 14 '18 at 20:03
  • You don't get conversion for template matching, other than derived to base. Please post a **complete example** that shows the issue. – Cheers and hth. - Alf Feb 14 '18 at 20:06
  • is there any reason you dont use `std::valarray` ? – 463035818_is_not_an_ai Feb 14 '18 at 20:09
  • @user463035818 std::valarray is dynamically allocated and not fixed size. – eike Feb 14 '18 at 20:11
  • **−1** Not the real code (e.g. lacks constructor definitions). – Cheers and hth. - Alf Feb 14 '18 at 20:11
  • I would vote to **close this question** as lacking reproducible example except that I clicked the wrong reason first, and now SO won't let me but instead revels in showing number of seconds since I did that. Argh. Anyway. – Cheers and hth. - Alf Feb 14 '18 at 20:14
  • @eike See [Prefer composition over inheritance?](https://stackoverflow.com/q/49002/1896169) – Justin Feb 14 '18 at 20:14
  • @Cheersandhth.-Alf Yes, and that is apparently the problem. I'm sorry for not including the constructor, I was too short sighted to expect a problem coming from there, but it in fact did. I will update the question shortly and include the answer. – eike Feb 14 '18 at 20:16
  • @Justin Since I want to expose all of the std::array interface, inheritance seems to be the superior option to me. – eike Feb 14 '18 at 20:24
  • The added constructor is better, but it's currently `private`, so this is still not the **real code**. – Cheers and hth. - Alf Feb 14 '18 at 20:24
  • @eike In almost all cases, it's still wrong. If your `Vec` isn't supposed to be thought of as a `std::array`, even if you want the same interface, just rewrite the interface in `Vec`. Especially for `std::array`, there aren't that many functions to write out. – Justin Feb 14 '18 at 20:25
  • @Cheersandhth.-Alf Copy&Paste error. In the compiler explorer link it is public. I did not use my actual code because there is added complexity that (unlike the constructor) does not have any impact on the problem. I rushed the edits because I didn't want anyone to be mislead by the incorrect example I originally gave. – eike Feb 14 '18 at 20:28
  • OK. Currently the constructor specifies an implicit conversion from single `T` to array. You can make the argument a `std::initializer_list` to avoid that implicit conversion. Then fix the rest. – Cheers and hth. - Alf Feb 14 '18 at 20:34
  • @Cheersandhth.-Alf My original ctor used std::initializer_list, but I replaced it because of the overhead std::initializer_list brings. – eike Feb 14 '18 at 20:36
  • Make things correct first. Only then optimize. If measurements tell you that optimization is really necessary. – Cheers and hth. - Alf Feb 14 '18 at 20:37
  • @Cheersandhth.-Alf Solid advice. Would +1, if I could. – eike Feb 14 '18 at 20:41

1 Answers1

0

Add another operator* overload function where the LHS can be a number.

template<class S>
   friend Vec<T, N> operator*(S b, Vec<T, N> a)
   {
      return a *= b;
   }
R Sahu
  • 204,454
  • 14
  • 159
  • 270
  • As Justing pointed out in the comments, this should work. With the original problem being the ctor, is it still correct to accept this answer or would it be better to delete the question? – eike Feb 14 '18 at 20:38
  • @eike, That's your call. – R Sahu Feb 14 '18 at 20:43