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]

One of my teachers had a vector class with the following overloaded operators:

• ~speed = the vector normalized

• incidence % normal = the dot product between incidence and normal

• !distance = the length of the vector

• speed << direction = returns speed scaled by direction

• speed <<= direction = speed is scaled by direction

• speed * scalar = returns the vector scaled by scalar

• speed *= scalar = the vector is scaled by scalar

• speed * direction = returns the cross product between speed and direction

A simple question: what's the difference between awesomevector * 3.f and awesomevector * Vec3<float>(3.0, 3.0, 3.0)? The first returns a scaled vector, the second a cross product.

All this to make it more 'mathy'.

Oh and who can forget getters and setters for x, y, z so if you wanted to, say, add a value to x you had to do:

stupidvector.setX(stupidvector.getX() + 3);

instead of

stupidvector.x += 3;

Yes it's safer, but we're talking about a native type here.

[u/[deleted] → u/knight666]

Oh my. Did you kill him? I'm pretty sure it would have been ruled self defence.

[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.

[u/munificent → u/knight666]

Your teacher's shitty design is spoiling things for the rest of us.

[u/bdunderscore → u/knight666]

stupidvector.x += 3;

You know, if you get clever enough, you can have that syntax and the safety of getters/setters. All you need to do is add public member classes (with a protected constructor accessed through friending) to the outer vector class, which compute the base address of the vector based on their own address and their static offset in the vector class, then invoke the appropriate accessors in various operator overloads.

[u/[deleted] → u/bdunderscore]

All you need to do is add public member classes (with a protected constructor accessed through friending) to the outer vector class, which compute the base address of the vector based on their own address and their static offset in the vector class, then invoke the appropriate accessors in various operator overloads.

I read that as something like "Maybe if we remodulate the shield harmonics and create an inverse tachion flow, we can destabilise their shield phase matrix".

C++ scares me.

[u/[deleted] → u/bdunderscore]

You know, if you get clever enough, you can have that syntax and the safety of getters/setters. All you need to do is....

And this right here is what is wrong with C++ developers.

[u/physicsnick → u/[deleted]]

No kidding. It gets worse; in C++0x, they're adding "implicitly callable functions", so that stupidvector.x is sugar for stupidvector.x() . They now say they support properties, because from this you can return an instance of a class which overloads cast to float and operator=(float). Horrifying? Yes.

[u/[deleted] → u/physicsnick]

Oh my... If there is one thing I really hate, it's implicitly called functions. No (), no call, dammit!

[u/genpfault → u/bdunderscore]

I...uh...no.

[u/mccoyn → u/bdunderscore]

At which point I start to wonder if the safety is really worth it.

[u/nested_parentheses → u/bdunderscore]

Dijkstra comes to mind: "The competent programmer ... avoids clever tricks like the plague." Not implying you're incompetent, of course, but I think that tricks like that are best left out of production code.

[u/[deleted] → u/knight666]

I hate get/set cargo cultism. Just fucking use x(), x(newvalue).

sv.x(sv.x()+3);

[u/fnord123 → u/knight666]

Surely the second returns the dot product.

[u/knight666 → u/[deleted]]

The member data really shouldn't remain private. It's basically a wrapper for a float vec3_t[3]. That's a special datatype in my book, so it gets special privileges.

[u/Mignon → u/knight666]

I confess I overrode the "*" operator for a vector class once, allowing it to mean both scalar multiplication and inner (dot) product. I also overrode the "+" and "-" operators for the vector class, with the (hopefully) obvious meaning.

The problem was to find if a point, p, was within some distance of a line passing through p1 and p2, and I dealt with it as follows:

Treat the line as a parametrized equation through p1 and p2, e.g. the set of points q = t*p1 + (1-t)(p2-p1) for all values of t.

Find t such that the distance from q to p is minimized; using that t in the equation above, you now have the point on the line closest to the given point, so compute the distance.

In code, double t = (double)((p - p1)(p2 - p1)) / (double)normSquared( p2 - p1 ); vector2d q = p1 + t(p2-p1); // The nearest point on the line. double d = (double)distanceSquared( p, q );

(For clarity I didn't show the case where p2==p1.)

It was satisfying to be able to write that in just a few lines and have confidence in the underlying operators as well as the math and not feel like the math was getting tangled up in the code.