Return to Answer

• 1

• 10.6k
• 2
• 31
• 51

• 10.6k
• 2
• 31
• 51

Looks good over all, but I have a few thoughts.

I would use the value_type typedef for method return types, temporary types, etc. It should make for a bit prettier signature in IDEs and any kind of auto generated documentation.

I would try to be consistent with variable names with regards to N/M/r/c. I either use N/M/n/m or R/C/r/c. Some people like to avoid using different capitalizations though, so that might be a valid concern.

Template type parameters default to the current ones inside of a templated class, so all of the FixedSizeMatrix<Ty, M, N> types can be simplified to FixedSizeMatrix.

It's good that you're consistent, but 2 spaces is rather unusual for C++. I would use either 4 spaces or tabs.

You should also have a const version of operator().

If you ever decide to make the class more sophisticated, you might regret exposing element. Perhaps hide the struct as a private member, have operator() be unchecked, and have a checked version called at. You could even have a 1 parameter version of each one that would provide the flattened functionality.

This unfortunately would break aggregate initialization though, which I suspect is why you've made the decision to have it public.

Still though, even if you make keep it public, you can get in a habit of using operator() and at() and then if you ever use C++11, you can properly encapsulate it with minimal code breakage.

I would throw a std::out_of_range instead of using asserts in operator(). Note that this and having the checked version named at while using the operator as a pass through is consistent with the behavior of std::vector.

Same thing for your other asserts. Exceptions can be caught and handled if desired. Asserts not so much. if you're using asserts for performance reasons, please consider if the tiny difference really matters. Modern C++ compilers have a zero cost exception model where exceptions are only costly if they're actually thrown.

Don't use double underscores. You technically can in certain situations, but the rules are very difficult to remember. Your FIXED_SIZE_MATRIX_H__ is technically invalid.

Is I common notation for a matrix inverse (makes me think identity matrices)? Even if it is, I would name the method invert.

Same thing with the transpose.

Any time you're passing the matrix as an argument to a function and you're not modifying it, pass it by constant reference. There's no need to make a potentially very expensive copy just to read from it. There's also the option of having the argument copy but then using the copy to store the result (though that requires that you can operate on the matrix without a dependency cycle between elements). This would allow move semantics in C++11 on temporaries passed as the argument.

Everywhere you do Ty(0), you force an interface constraint on Ty in that it must have a constructor that is compatible with a single integer parameter. If you default construct the Ty, you avoid this interface constraint, though you of course add a new one. Consider the two cases this can affect: is a type used more likely to have a default constructor that represents 0 or a single integer constructor? I would lean towards the first one, oddly enough. For example, a Complex might not have an integer constructor.

It might be nice to put this in a namespace if you plan on the usage of this to grow to more than just 1 or 2 simple programs. That helps mentally organize your library as one coherent part, and it provides a few technical advantages. Namely, your detail namespace could conflict with someone else's ::detail namespace, and it would be nice to give your macros, though short lived, a prefix of some kind. It's relatively unlikely that anything in this would actually conflict, but if someone were using this with another matrix library, you never know. Namespaces don't cost anything to you as the developer (or to consumers of the class), but they offer a few advantages.