ethz-asl / programming_guidelines

This repository contains style-guides, discussions, eclipse/emacs auto-formatter for commonly used programming languages
139 stars 38 forks source link

Names representing template parameters should a single letter in case there are few arguments and their meaning is clear. Otherwise the parameters should be prefixed by the parameter type and contain a mixed-case word describing the meaning of the paramet #3

Closed simonlynen closed 11 years ago

simonlynen commented 11 years ago
template<typename T> ...
template<int I_ROWS, bool B_RowMajor, typename F_SortingFunctor, typename T_ValueType> ...
HannesSommer commented 11 years ago

Underscore necessary?

I would prefer the same without the underscore, because to me it seems unnecessary work. The single letter prefix clearly is still an eye catcher without it, because two capital letters in the beginning should never happen in type names at least if one obeys https://github.com/ethz-asl/programming_styleguide/wiki/Cpp-Coding-Style-Guidelines#10-abbreviations-and-acronyms-must-not-be-uppercase-when-used-as-name-4 and single letter fragments are avoided, which they should be anyway:

template<int IRows, bool BRowMajor, typename FSortingFunctor, typename TValueType>

(of course I had to fix here ROWS to Rows, which is the correct way according to the rule with or without a underscore.)

Prefix 'F' conflict

There is a conflict with function pointer template parameters. But I also like the idea of having a separate prefix for functors (although they are of course a special case of typenames). Any idea how to solve this conflict? Maybe we could use 'FP' for function pointers instead because they are probably rarer then functor parameters. Another probably cleaner solution would be to have functor as an postfix and then keep T as prefix for functors. In the example there is actually some redundancy. I would vote for the latter. It would look for example the following way :

template<typename TSortingFunctor>

template<typename TElement, int (*FCompare)(const TElement &, const TElement &)>
simonlynen commented 11 years ago

Hannes,

the longer I think about the prefix, the more I actually think it is unnecessary. It makes perfekt sense if there are only single letter parameters.

But, what do we need the B for if the template type is bool anyway, what the I for if the argument is of type integer

Similarly for types and functors: If the programmer follows the rule giving meaningful names for the parameters, it should be clear without the prefix whether a parameter is a type or not. Just as in the example that you gave.

Therefore I think we are probably better off without the additional prefix. What do you think?

Simon

stephanemagnenat commented 11 years ago

I am for scratching prefixes... :-)

HannesSommer commented 11 years ago

Hm, I don't think to have different prefixes is of great importance. It would be clearly of some use, though because you don't have the template parameter type written where you actually use them. And there is a lot of important and fundamental difference between using e.g. template-template parameters or integers. And to indicate this difference always as part of name in a non standardized way just makes the names longer, than these single character prefix would do.

But much more of importance seems to me to have at least some common prefix or suffix for the separation of identifier worlds. Just as it is very important to separate the type names from the variable names (that's the former start with a capital and the latter with a lower case letter) it is even more important to separate type names and constant names from template parameter names. Think e.g. of the quite common and important situations:

typedef TElement Element; 

or

enum { Rows = IRows };  

And that was the original idea of introducing some kind of prefix or suffix. Because by just the case of the first letter we can only distinguish two identifier worlds and not three! So if we don't want to have prefixes or suffixes for both typedefs, constants and template parameters, how do we give an easy solution to the upper situations?

stephanemagnenat commented 11 years ago

In the situation you mention, do you really want to redefine things, in the same namespace, with different names? I perfectly understand typedefs like typedef std::vector<int> MyVector and ones like typedef Eigen::Matrix MyMatrix, but within the same name space, if there are many redefinitions, isn't it showing some design problem? In general, I think that it is important to be as clear and simple as possible.

HannesSommer commented 11 years ago

The thing is that you typedef the value of the template paramter to access it later (from outside). The template parameters value is no name in the namespace of the template class, right? That is why e.g. Eigen::Matrix has the enum RowsAtCompiletime set to the value of the template argument _Rows: http://eigen.tuxfamily.org/dox/Matrix_8h_source.html#l00113. But in my opinion how they solve it is just bad. They essentially call quite the same things very differently. I would prefer to call them in a systematic way quite similar as it is sadly not possible to call them the same in general - at least as far as I know.

stephanemagnenat commented 11 years ago

Yes you are right, I forgot about the context, my mistake.

simonlynen commented 11 years ago

Exporting types of template parameters via a typedef is commonly used in our office as well.

Yes same name is not possible due to the ambiguity. If we write the template parameters capitalized then the typedef could be written mixed case following the rule for types. However capitalized names would potentially conflict with macros. So I think this is not an option.

stephanemagnenat commented 11 years ago

Then why not using a heading or trailing underscore, but with the same name as the typedef, for template parameters when an export is required, and no underscore otherwise?

HannesSommer commented 11 years ago

This sound good to me. I would vote a postfix underscore because the prefix is commonly reserved for system / language libraries or compiler specific names. So we would have :

typedef Element_ Element; 

or

enum { Rows = Rows_ };  

Looks quite ok. Yes, with that I would be happy after a while... great!

The only problem I see is in the rule part "if necessary". This would mean you have to add underscores at some places when you later decide to export the parameters value. Why not generally add the postfix underscore to template parameter names?

simonlynen commented 11 years ago

Yes generally using postfix underscore seems reasonable. This does also not conflict with member variables. (Capital first letter)

So is this one agreed upon?

furgalep commented 11 years ago

Subtle point: The enum in the above example would have to be

enum { ROWS = Rows_ };

(based on https://github.com/ethz-asl/programming_styleguide/issues/1)

right?

I'm still thinking about this one. I don't mind it but I'm hesitating because I've never seen it this way before.

HannesSommer commented 11 years ago

Yes, seems you are right with the upper case :). Sorry - wasn't up to date with that one...

About the usage of the underscore for template names: Eigen seems to use at least sometimes the underscore prefix : http://eigen.tuxfamily.org/dox/Matrix_8h_source.html

simonlynen commented 11 years ago

@HannesSommer could you integrate that to the styleguide? Thanks

HannesSommer commented 11 years ago

Adapted the wiki.