Skip to main content
Code Review

Return to Answer

replaced http://stackoverflow.com/ with https://stackoverflow.com/
Source Link
  • You're using a reserved naming convention. The Standard Library and compiler-related extensions reserve names starting with _ followed by an uppercase letter for internal use in macros, private types or even global variables. See the anser at: What are the rules about using an underscore in a C++ identifier? What are the rules about using an underscore in a C++ identifier?

  • Probably a typo, but if you're casting to vec2, then I suppose the intended was to return a vec2? If you never actually use that method in the code, it will never get instantiated, because this is a template class, so the compiler will not report this kind of typos that it would otherwise flag in non-template code.

  • You're using a reserved naming convention. The Standard Library and compiler-related extensions reserve names starting with _ followed by an uppercase letter for internal use in macros, private types or even global variables. See the anser at: What are the rules about using an underscore in a C++ identifier?

  • Probably a typo, but if you're casting to vec2, then I suppose the intended was to return a vec2? If you never actually use that method in the code, it will never get instantiated, because this is a template class, so the compiler will not report this kind of typos that it would otherwise flag in non-template code.

  • You're using a reserved naming convention. The Standard Library and compiler-related extensions reserve names starting with _ followed by an uppercase letter for internal use in macros, private types or even global variables. See the anser at: What are the rules about using an underscore in a C++ identifier?

  • Probably a typo, but if you're casting to vec2, then I suppose the intended was to return a vec2? If you never actually use that method in the code, it will never get instantiated, because this is a template class, so the compiler will not report this kind of typos that it would otherwise flag in non-template code.

Source Link
glampert
  • 17.3k
  • 4
  • 31
  • 89
template <typename _L> operator vec2<_L>() const { return vec3<_L>(x, y); };

Two issues with this line:

  • You're using a reserved naming convention. The Standard Library and compiler-related extensions reserve names starting with _ followed by an uppercase letter for internal use in macros, private types or even global variables. See the anser at: What are the rules about using an underscore in a C++ identifier?

  • Probably a typo, but if you're casting to vec2, then I suppose the intended was to return a vec2? If you never actually use that method in the code, it will never get instantiated, because this is a template class, so the compiler will not report this kind of typos that it would otherwise flag in non-template code.


This comparison operator seems very convoluted and, to some extent, questionable:

bool operator<(const vec3& other) const {
 if (x < other.x)
 return true;
 if (x > other.x)
 return false;
 //x == other.x
 if (y < other.y)
 return true;
 if (y > other.y)
 return false;
 //y == other.y
 if (z < other.z)
 return true;
 if (z > other.z)
 return false;
 //z == other.z
 return false;
};

If a single element of this vector is less than other, you return true. I have doubts if this is a good definition of operator <. The usual would be to only consider this < other if all members of this are less than other. In some cases it might also make more sense to test the lengths/magnitudes of the vectors.

bool operator < (const vec3& other) const
{
 return (x < other.x) && (y < other.y) && (z < other.z);
 // or alternatively:
 // return sqrMagnitude() < other.sqrMagnitude();
}

The difference between invert and inverse is not clear without looking at the implementation. They actually do the same thing, only difference is that invert mutates this. If you really think there's a need for the existence of the two methods, consider renaming inverse to inverted, being consistent with the other notation used on normalize-normalized.

But actually, inverting a vector is usually associated with changing its direction, that is, negating the vector. Your invert function performs 1/vec, which is an operation usually associated with the reciprocal of a value. I suggest a complete name change for that method.


Final nitpicking, don't add ; to the end of each function. It is unnecessary and if you try to compile on Clang or GCC with maxed-out warning levels, you'll get a torrent of warnings for the extra semicolons.

lang-cpp

AltStyle によって変換されたページ (->オリジナル) /