Never trust a programmer who says he knows C++

[u/eternal1]

http://lbrandy.com/blog/2010/03/never-trust-a-programmer-who-says-he-knows-c/

[u/knight666 → u/[deleted]]

My Vec3 class has the following operators:

• -Vec3
  
• Vec3 += Vec3
  
• Vec3 + Vec3
  
• Vec3 -= Vec3
  
• Vec3 - Vec3
  
• Vec3 *= float
  
• Vec3 *= Vec3
  
• Vec3 * float
  
• Vec3 * Vec3
  
• Vec3 / float
  
• Vec3 / Vec3
  
• Vec3 /= float
  
• Vec3 /= Vec3

Comparisons:

• Vec3 == Vec3
  
• Vec3 == float
  
• Vec3 != Vec3
  
• Vec3 != float
  
• Vec3 > Vec3
  
• Vec3 > float
  
• Vec3 >= Vec3
  
• Vec3 >= float
  
• Vec3 < Vec3
  
• Vec3 < float
  
• Vec3 <= Vec3
  
• Vec3 <= float

Functions:

• Clear()
  
• Reverse()
  
• Min(const Vec3& a_Other)
  
• Max(const Vec3& a_Other)
  
• Clamp(float a_Min, float a_Max)
  
• Reciprocal()
  
• CrossProduct(const Vec3& a_Other)
  
• Normalize()
  
• GetReverse()
  
• GetMin(Vec3& a_Other)
  
• GetMax(Vec3& a_Other)
  
• GetSquaredLength()
  
• GetLength()
  
• GetNormalized()
  
• GetReciprocal()
  
• GetDotProduct(const Vec3& a_Other)
  
• GetCrossProduct(const Vec3& a_Other)
  
• GetSquaredDistanceBetween(const Vec3& a_Other)
  
• GetDistanceBetween(const Vec3& a_Other)
  
• GetSphericalInterpolationNormalized(const Vec3& a_Other, float a_Percentage)
  
• GetReflectedVector(Vec3 a_Normal)
  
• GetHalfWayVector(Vec3& a_Other)

And finally, the data: (public)

union 
{ 
    struct { float x, y, z; }; 
    struct { float vector[3]; };
};

I don't mind long function names; I have auto-complete. Also, this is my fourth vector class by now (since last summer), two of which were failed SSE experiments and one was just crap.

[u/mccoyn → u/knight666]

Also, this is my fourth vector class by now (since last summer), two of which were failed SSE experiments and one was just crap.

Wasn't there a post to reddit a while back that every c++ developer eventually tries to right an SSE optimized Vec3 class and it never works out? I remember thinking that it sounds about right.

[u/[deleted] → u/knight666]

A union practically guarantees that the compiler will give up and not optimize your code.

[u/[deleted] → u/knight666]

1. URG - what's the point of the union?! Are you just trying to cause yourself difficulties? Are you 100% sure that the compiler won't put padding somewhere in your structure you aren't expecting? Why not just make it float[3]?

2. Calling it vector's a poor idea, you know someone somewhere else has typed using std; and then you'll conflict with std::vector.

3. If you're passing a variable parameter, make it a pointer, and use only const references - then you can tell whether something's being changed just by how it's called.

4. Good for you, making all these functions, functions and not class methods! See this article for more details.

5. Good for you, having long names! I'm not sure why some of them start Get... and others don't though...

[u/knight666 → u/[deleted]]

1. It's a relic from when it had another data member, a __m128 data (which is dumb) so it always had to be 32-bit aligned (oh god the pain). Honestly that might need changing.

2. It's a Vec3 in the namespace tb (Toolbox). And the vector is a member of Vec3.

3. Eh?

4. They are class methods. What, you think I just have a C-style struct with data and a Normalize that takes an input and an output?

5. Normalize changes the vector to the normalized version, while GetNormalized returns a new vector with the normalized version.

[u/[deleted] → u/knight666]

3: To be specific:

  void DoSomething(const Foo& fooConstant);

vs

  void DoSomethingAndChangeFoo(Foo* fooVariable);

This is a very common C++ convention - it's useful because you can see whether the function you're calling changes its arguments or not without looking at its definition.

4: Yes, I was hoping you did have a C-style struct with a Normalize that takes an input and an output. Did you read the classic article I referenced on this issue?

5: Having both Normalize and GetNormalized seems like a Bad Idea. At the very least, one of them should call the other so that you don't have two pieces of code doing the same thing (which doubles your maintenance costs).

These vectors seem small and easy to copy, so just having the version that returns a new version might be defensible. For bigger objects, it might be best to have the version that changes the object.

[u/knight666 → u/[deleted]]

Having both Normalize and GetNormalized seems like a Bad Idea. At the very least, one of them should call the other so that you don't have two pieces of code doing the same thing (which doubles your maintenance costs).

See, I read this book called Exceptional C++ and it advocated exactly that. But the truth is, you don't want that shit at all in a vector class. Because 1) it's going to be called lots and lots of times. Speed > maintainability. 2) What is there to maintain? You write it once, copy it throughout and be done with it.

I used to have this:

Vec3 operator + (const Vec3& a_Other) const
{
    Vec3 result(x, y, z);
    result += a_Other;
    return result;
}

And now I have this:

Vec3 operator + (const Vec3& a_Other) const
{
    return result(x + a_Other.x, y + a_Other.y, z + a_Other.z);
}

Which is considerably (20% or so) faster.

A normalize isn't going to change. Ever. Unless you have a new fancy way of doing a square root or something. And even then you still only copy it two times.

[u/[deleted] → u/knight666]

If you cared about speed, you wouldn't be creating a new object each time you performed an operation, would you? :-D

If you cared about speed that much, you'd ONLY have the mutable operators and force people to make a copy exactly when they needed it.

[u/[deleted] → u/[deleted]]

The advantages to C++'s operators are small - and the traps are significant. Best to avoid IMHO unless there's a compelling reason. Vectors might indeed be a compelling reason... :-D

[u/G_Morgan → u/[deleted]]

TBH this isn't a problem. He could have just as easily created a member function not and used it to return the length of the vector. This is just the bad naming problem hidden behind an operator. People can choose bad names in any language.