Show Posts
1
General / Re: ExMa2D [C++14] (feedback)
« on: March 08, 2016, 08:22:21 pm »
Sorry for the late response, have been a bit busier than expected. Anyway, here it is.
I think I have imposed pretty strict rules on types what the functions can take. I can't imagine a scenario where a function would be inadequately generated for unwanted types. Would you mind providing such a scenario?
I have missed a part of your previous post, Hiura, so here's my response:
So even when it's a made up name, IMHO it does the job the best. Just my preference, really. (want to be as transparent as possible )
Some comments:An excellent suggestion! Thanks, I will work on this in the near future.
The constraint that the class must have x and y members is unnecessary. Have a look at Boost.Geometry to see how is possible to write fully generic, yet simple to use code. Basically, the idea is:namespace lib
{
// user can specialize this class to support other types
template <typename T>
struct vector_type
{
static auto x(const T& v) { return v.x; }
static auto y(const T& v) { return v.y; }
};
// convenience inside library, can also be in private namespace
template <typename T>
auto x(const T& v)
{
return vector_type<T>::x(v);
}
}
Does SFINAE based on std::is_arithmetic actually bring an advantage? Types that don't provide the operations won't compile already, and you don't artificially limit what types users can work with.Yeah, kinda. I am using std::numeric_limits<T>::epsilon() in the compare(), which doesn't make it an artificial limitation, but a real one. Of course, I don't call compare() in every function, do you think I should remove this constraint from those functions? Or should I implement compare() with a fixed allowed deviation?
Your functions take their parameters by value, leading to unnecessary copies.Thanks, fixed.
The excessive type inference makes code a bit difficult to understand sometimes, and I'm not sure if it does what's intended all the time. Use it where it helps increase genericity, but in cases where you know the type, it may be helpful to write it down (even function-local as typedefs from metafunctions).Not really great with terminology, do you mean the unnecessary auto with decltypes? If so, it has been fixed as well (in the commit above).
You can use macros to increase the readability of SFINAE. In places where it doesn't contribute to overload resolution, use static assertions instead of SFINAE, in order to create more meaningful error messages and not clutter the API.I don't really see how would this help. Would you explain yourself a bit more clearly with an example? (I am obviously a beginner in these manners, so I tend to be a bit annoying with my lack of knowledge. Feel free to ignore this, if you find it annoying)
Operators like unary - are not helpful inside your namespace, because they're not considered by ADL unless a library type is used as an argument. The only way to use them is to extract them to the global namespace, which is a very bad idea, because a function template without constraints accepts everything, and it will defeat implicit conversions of completely unrelated code.This is meant to be like this. I don't want to be invasive, so if user has already defined basic operators, and only wants to use higher-level functions, (s)he is free to do so.
I think I have imposed pretty strict rules on types what the functions can take. I can't imagine a scenario where a function would be inadequately generated for unwanted types. Would you mind providing such a scenario?
The function std::abs() is part of the standard library, and it's possibly more efficient than yours.It surely is. But it is not constexpr by standard (g++ does implement it as constexpr, but clang doesn't.) Should I give up constexpr in abs, which means in compare as well, which means in a bunch of other functions as well?
utils::compare() should have a better name.True. What would you suggest? is_equal?
It definitely does, it's common convention in C++ to use .cpp for translation units (= files that are compiled). AndFixed. Using .tpp from now on.Hiuradabbertorres Yeah, my bad. Sorry. is also right that IDEs work according to that scheme.
90 degrees are 100 gradians, but pi/2 radians.I didn't mean to imply that 100 rad is 90 deg (although it could have been understood like that, good catch!), I just used arbitrary values. Anyway, didn't know about gradians, thanks for the info!
Note that those last quotes were not from me but from dabbertorres.Sorry, using phone is not the best way how to write comprehensive posts. It has been fixed.
I have missed a part of your previous post, Hiura, so here's my response:
I use `normal` in my codebase actually. It's a noun/adj, not a verb, but it makes it clear enough I believe. One could also expect `orthogonal` since it's mostly a synonym.I like to use verb as function names, and normal could be mixed up with normalize, and the similarity doesn't really contribute to IntelliSense as well. Also orthogonize is no better
So even when it's a made up name, IMHO it does the job the best. Just my preference, really. (want to be as transparent as possible )
