I see a number of things that may help you improve your program.
Include all required headers
The template has calls to acos
and sin
but doesn't include the <cmath>
header which is required. The code should have this line:
#include <cmath>
Use namespaces
The previously mentioned acos
and sin
should use the namespace prefix as std::acos
and std::sin
.
Don't use anonymous struct
s
While they are standard in C11, they are not allowed in C++. There are compiler extensions that can enable them but they don't conform to the standard they don't conform to the standard. In this case, you could either name it or use the next suggestion.
Eliminate the union
Within the array is never used. This suggests that the union could simply be eliminated. If you think that users of the code are going to want to refer to the elements as an array, you could provide an operator[]
implementation for that with cleaner syntax anyway.
Eliminate vec2
The vec3
class does not seem to rely on a vec2
for existence or functionality, so it should not be a part of the class definition. Right now, it's not possible to use the class unless one has a vec2
also available, but that dependency is also not #include
d per the first suggestion.
Don't use #pragma once
Although it is supported by some compilers, code which is intended to be reused should avoid non-standard extensions. By definition, all #pragma
are non-standard. For portable code, you should use the standard include guards use the standard include guards. Even if you are only ever using one compiler at the moment, you will want to know the portable way of accomplishing this.
Think about unsigned
numbers
It may be that you intended for this class to use exclusively floating point numbers, but there isn't any checking of that, so the result is that this line:
std::cout << vec3<unsigned>::left.toString() << '\n';
compiles and runs just fine, but gives a peculiar result:
[x: 4294967295, y: 0, z: 0]
I would recommend adding a static_assert
to the class:
static_assert(std::is_signed<T>{}, "Error: vec3 type must be signed");
Use appropriate floating point types
If I am using a vec3<double>
, I might not be happy that angle
is internally using only a float
instead. Consider using either T
or double
where float
is currently used within the class member functions.
Wrap it in a namespace
To make sure your vec3
doesn't collide with another in some other library, you might want to wrap the whole thing up in your own namespace.
I see a number of things that may help you improve your program.
Include all required headers
The template has calls to acos
and sin
but doesn't include the <cmath>
header which is required. The code should have this line:
#include <cmath>
Use namespaces
The previously mentioned acos
and sin
should use the namespace prefix as std::acos
and std::sin
.
Don't use anonymous struct
s
While they are standard in C11, they are not allowed in C++. There are compiler extensions that can enable them but they don't conform to the standard. In this case, you could either name it or use the next suggestion.
Eliminate the union
Within the array is never used. This suggests that the union could simply be eliminated. If you think that users of the code are going to want to refer to the elements as an array, you could provide an operator[]
implementation for that with cleaner syntax anyway.
Eliminate vec2
The vec3
class does not seem to rely on a vec2
for existence or functionality, so it should not be a part of the class definition. Right now, it's not possible to use the class unless one has a vec2
also available, but that dependency is also not #include
d per the first suggestion.
Don't use #pragma once
Although it is supported by some compilers, code which is intended to be reused should avoid non-standard extensions. By definition, all #pragma
are non-standard. For portable code, you should use the standard include guards. Even if you are only ever using one compiler at the moment, you will want to know the portable way of accomplishing this.
Think about unsigned
numbers
It may be that you intended for this class to use exclusively floating point numbers, but there isn't any checking of that, so the result is that this line:
std::cout << vec3<unsigned>::left.toString() << '\n';
compiles and runs just fine, but gives a peculiar result:
[x: 4294967295, y: 0, z: 0]
I would recommend adding a static_assert
to the class:
static_assert(std::is_signed<T>{}, "Error: vec3 type must be signed");
Use appropriate floating point types
If I am using a vec3<double>
, I might not be happy that angle
is internally using only a float
instead. Consider using either T
or double
where float
is currently used within the class member functions.
Wrap it in a namespace
To make sure your vec3
doesn't collide with another in some other library, you might want to wrap the whole thing up in your own namespace.
I see a number of things that may help you improve your program.
Include all required headers
The template has calls to acos
and sin
but doesn't include the <cmath>
header which is required. The code should have this line:
#include <cmath>
Use namespaces
The previously mentioned acos
and sin
should use the namespace prefix as std::acos
and std::sin
.
Don't use anonymous struct
s
While they are standard in C11, they are not allowed in C++. There are compiler extensions that can enable them but they don't conform to the standard. In this case, you could either name it or use the next suggestion.
Eliminate the union
Within the array is never used. This suggests that the union could simply be eliminated. If you think that users of the code are going to want to refer to the elements as an array, you could provide an operator[]
implementation for that with cleaner syntax anyway.
Eliminate vec2
The vec3
class does not seem to rely on a vec2
for existence or functionality, so it should not be a part of the class definition. Right now, it's not possible to use the class unless one has a vec2
also available, but that dependency is also not #include
d per the first suggestion.
Don't use #pragma once
Although it is supported by some compilers, code which is intended to be reused should avoid non-standard extensions. By definition, all #pragma
are non-standard. For portable code, you should use the standard include guards. Even if you are only ever using one compiler at the moment, you will want to know the portable way of accomplishing this.
Think about unsigned
numbers
It may be that you intended for this class to use exclusively floating point numbers, but there isn't any checking of that, so the result is that this line:
std::cout << vec3<unsigned>::left.toString() << '\n';
compiles and runs just fine, but gives a peculiar result:
[x: 4294967295, y: 0, z: 0]
I would recommend adding a static_assert
to the class:
static_assert(std::is_signed<T>{}, "Error: vec3 type must be signed");
Use appropriate floating point types
If I am using a vec3<double>
, I might not be happy that angle
is internally using only a float
instead. Consider using either T
or double
where float
is currently used within the class member functions.
Wrap it in a namespace
To make sure your vec3
doesn't collide with another in some other library, you might want to wrap the whole thing up in your own namespace.
I see a number of things that may help you improve your program.
Include all required headers
The template has calls to acos
and sin
but doesn't include the <cmath>
header which is required. The code should have this line:
#include <cmath>
Use namespaces
The previously mentioned acos
and sin
should use the namespace prefix as std::acos
and std::sin
.
Don't use anonymous struct
s
While they are standard in C11, they are not allowed in C++. There are compiler extensions that can enable them but they don't conform to the standard. In this case, you could either name it or use the next suggestion.
Eliminate the union
Within the array is never used. This suggests that the union could simply be eliminated. If you think that users of the code are going to want to refer to the elements as an array, you could provide an operator[]
implementation for that with cleaner syntax anyway.
Eliminate vec2
The vec3
class does not seem to rely on a vec2
for existence or functionality, so it should not be a part of the class definition. Right now, it's not possible to use the class unless one has a vec2
also available, but that dependency is also not #include
d per the first suggestion.
Don't use #pragma once
Although it is supported by some compilers, code which is intended to be reused should avoid non-standard extensions. By definition, all #pragma
are non-standard. For portable code, you should use the standard include guards. Even if you are only ever using one compiler at the moment, you will want to know the portable way of accomplishing this.
Think about unsigned
numbers
It may be that you intended for this class to use exclusively floating point numbers, but there isn't any checking of that, so the result is that this line:
std::cout << vec3<unsigned>::left.toString() << '\n';
compiles and runs just fine, but gives a peculiar result:
[x: 4294967295, y: 0, z: 0]
I would recommend adding a static_assert
to the class:
static_assert(std::is_signed<T>{}, "Error: vec3 type must be signed");
Use appropriate floating point types
If I am using a vec3<double>
, I might not be happy that angle
is internally using only a float
instead. Consider using either T
or double
where float
is currently used within the class member functions.
Wrap it in a namespace
To make sure your vec3
doesn't collide with another in some other library, you might want to wrap the whole thing up in your own namespace.