On 12/8/2015 1:25 PM, Phil Endecott wrote:
Adam Wulkiewicz wrote:
The formal review of Emil Dotchevski's QVM library begins today on 7th Dec and ends on 16th Dec. Full documentation is also viewable on Github: http://zajo.github.io/boost-qvm/
I find most of the identifiers too short. To give just a couple of examples: "transp" is used to mean "transpose". You save typing three letters, and get confusion with transparent, transport, etc. Then look at the names of some traits classes; elsewhere we have type_traits, allocator_traits, iterator_traits etc. all spelt out in full, but in qvm we have q_traits, v_traits and m_traits. I could go on but really almost every identifier is too short for my tastes.
I used an earlier version of the library several years ago, back when it was called "Boost.LA", and I found extremely short identifiers to be a concern too. I could understand going for `mat` and `vec` instead of `matrix` and `vector`, but not just `m` and `v`. For pretty much every other identifier, I would like to see a full blown word instead. I'm not sure we need the prefix nor the suffix. The operations shouldn't have the type on its name. If the operation depends on a concept (vector, matrix, quaternion) and
Le 09/12/2015 13:43, Agustín K-ballo Bergé a écrit :
the operation has no parameter of this type the function should have the
type as parameter
What would be wrong replacing
float vmag = mag(v);
float33 m = rotx_m<3>(3.14159f);
vref(v,YXZ) = rotx_m<3>(3.14159f) * v;
by
int mag = magnitude(v);
float33 m = rotate_x<M3>(3.14159f);
ref(v).YXZ() = rotate_x<M3>(3.14159f) * v;
Where M3 is a matrix with 3 rows, e.g.
using M3 = qvm::mat
I'm really not a fan of the old operator% and now operator, syntax. To me, (v,XY) looks like you're forming a row-vector with two elements. Is there a reason why these accessors can't be written with function syntax, i.e. XY(v) ? Or, for matrices, something like element<4,2>(m) rather than (m,A<4,2>) ?
The precedence issues are so bad with `operator,` that one has to pretty much always wrap it in parens, that makes all precedence issues go away. I think for that reason it's a better choice than `operator%`, that mostly just works and bit me over and over again. There are reasons against it too, for instance a missing include, a typo, a shadowing variable will turn a swizzling expression into a regular comma expression.
That said, if I have to write `(v,XY)` instead of `v.XY` I'd rather write `XY(v)` instead.
Clearly the operator, is too controversial. Isn't qvm::ref(v).XY() terse enough? Vicente P.S. ref result could depend on the associated traits of v and must use SFINAE