Difference between revisions of "Talk:Coding/Guidelines"

From UFO:AI
(Dev's discussion)
Line 125: Line 125:
 
*70
 
*70
 
** geever: We have NULL almost everywhere, and it tells me it's a pointer, so I would rather use NULL than 0.
 
** geever: We have NULL almost everywhere, and it tells me it's a pointer, so I would rather use NULL than 0.
 +
*** mattn: NULL is not valid in the c++ context in every case. foo(NULL) calls foo(int). which is not always what you want.
 
*71   
 
*71   
 
**bayo: should be what we have now TAB=4 (it is what i use here)
 
**bayo: should be what we have now TAB=4 (it is what i use here)

Revision as of 14:55, 6 July 2012

C++

Now that we have enabled C++, I've taken the task to organize the discussion about which coding guidelines we'll need for C++. My favourite reading regarding this aspect is John Lakos "Large scale C++ projects". Ouote: "It doesn't matter which style you choose; it's important to use the chosen style consistantly."
UFO:AI is only medium scale. But it doesn't hurt to comply to those rules. --Duke 00:28, 11 April 2012 (SAST)

Participants

This should be discussed among those who consider themselves as 'current permanent contributors'. It's all about how *we* want the code to look like. The idea is to restrict the voting to those who have to suffer the most. As there are more coding styles than coders in the world, this strategy avoids endless discussions. I went through that several times, believe me :(
Invitations go out to (alphabetically):
bayo, Duke, geever, mattn
...and as a 'technical advisor' Tron
Hmm. Do we really have that few ? :(

Strategies

We could

  • discuss and write our own guidelines from scratch (oh no)
  • adopt some published guideline (and eventually modify it a bit)
  • find a checker and/or beautifier tool and accept what it does.

I vote for the second option because the first would mean reinventing the wheel and I don't think any tool can check all the rules.

Suggestion

Googling for "coding guidelines C++" we find lots of them. I checked a few of them and found this one: http://geosoft.no/development/cppstyle.html

  • Duke's comment: With a few exceptions it describes pretty much what I did intuitively in the past. So I like it. The exceptions are rules # 28, 36, 37, 38, 40, 58, 62, 71 and 83. My favourite rule is #89. But let's see what you guys come up with.
  • Bayo's comments

Dev's discussion

  • 3
    • bayo: our struct use the convention 3 with a prefix in lower case class uiMyFunkyWidget. Maybe we can use namespace, if we decide to use it. But while we have not updated part of the code with it we should use our current naming convention. Anyway, we can use both.
    • Duke: the lowercase prefix would render rules 4 and 6 somewhat senseless. Also, except for the ui we don't have a consistent convention afaik.
      • Then ok for this rule, i thinking we have own rules about that. But then it is really need to use namespace, at least for "ui". Bayo 10:26, 21 April 2012 (SAST)
        • Uhm, don't get me wrong. Some subsystem-prefix can be useful. But it shouldn't start with lowercase. So it's "Ui" or "Uis", and "Cpg" for campaign maybe. But we should have a discussion what our subsystems are in the first place.--Duke 23:23, 23 April 2012 (SAST)
    • geever: Our type names are quite much recognizable, not a standard though. I would rather not use prefixes, but if we wanna prefix types, use the prefix we have for functions "UI_" and "CP_". I don't really like the "CpgMission_t" names, sound weird.
  • 10
    • bayo: more and more i dislike that way. IDE is quite nice now. It it not beautiful, displeasing, but if you need it to live, its ok
    • Duke: I'm insecure here.
    • geever: dislike
  • 11
    • bayo: more and more i dislike that way. IDE is quite nice now. It it not beautiful, displeasing, but if you need it to live, its ok
    • Duke: in general I dislike all those single letter pre- and suffixes. But the sideeffect for setters has some charm
    • geever: definietly NO. it's horror
  • 14
    • geever: I don't like long variable names at all, they give me more possibility to mistype them. :) Keep them as short as they can (while they are still understandable ofc.)!
  • 23, 24...
    • bayo: no idea
    • Duke: imho we don't need a convention for that, so omit those two rules
      • Ok, we can take it as a "may". Bayo 10:26, 21 April 2012 (SAST)
    • geever: 24: we use Idx at multiple locations already. we can ommit them
  • 28
    • Duke: I agree with the 2nd half of that rule. But I prefer "init" instead of "initialize" and "calc" instead of "calculate"
      • I usually prefer full names. Much more readable. Bayo 10:26, 21 April 2012 (SAST)
        • Oh, I only use that for maybe a dozen *standard* abbreviations like the above. --Duke 22:14, 23 April 2012 (SAST)
    • geever: vote for short names (I wrote this to points 18 and 20, but it has a dedicated rule :) )
  • 31
    • bayo: i prefer Color::RED
    • Duke: Color::RED looks a tad nicer, but requires discipline. COLOR_RED is forced by the compiler ;)
    • geever: I like our prefixed constants either
  • 34
    • bayo: what we have is ok, should we really need to update everything again? Easy to do anyway, then i dont care
    • Duke: imho .h and .cpp are ok.
    • geever: .h and .cpp
  • 36
    • bayo: no idea, it can be problematic when fast computation is need? but most of the time we can do it
    • Duke: Disagree with that rule. Simple getters and setters should imho be inlined anyway and thus stay in the .h file.
    • geever: However sometimes I find our cpp files overstuffed with code, splitting everything to files would make our life a nightmare -- even for you who use IDEs
    • Tron:
  • 37
    • bayo: i dont like that, i prefere less newline
    • Duke: Disagree. 80 chars stems from punchcard. We have reached the age of widescreens and zoomable editors. And we don't print the code, do we ? Suggestion: The code without the comments *should* not be wider than 60 chars (think of editing on a smartphone), the whole line *must* end before 140 chars are reached.
      • BTW i dont like coment in the same line. Bayo 10:26, 21 April 2012 (SAST)
    • geever: disagree, I won't count characters when typing
  • 38
    • bayo: i prefer TAB to align code, but i dont want special char in strings "\t" instead of " ", dont know what they talk about.
      Bayo, that rule is not about "\t". But that should be avoided too.--Duke 00:39, 19 April 2012 (SAST)
    • Duke: NO page breaks in the source. But I prefer tabs over spaces.
    • geever: our indent rule is good enough: use tabs, no page breaks
  • 39
    • geever: indent rest of the line by one (tab) but don't pad the code with spaces to the matching bracket like in the example
  • 40
    • Duke: Sure we need include guards like that. But our convention should be more like [DIR_[SUBDIR_]]CLASS_H
      • Yes, still use what we have. Bayo 10:26, 21 April 2012 (SAST)
    • geever: what we have now is good enough (don't need to overcomplicate it)
  • 45
    • geever: not sure if it should be done every time... annoying
  • 46
    • geever: our previous (maybe non-documented) rule was to initialize the variables the closest to their use, and I feel it more appropriate. However it is a good question where to declare variables...
  • 49
    • bayo: we are not making strong OOP software, then i dont care about this rule. struct or class is the same for me
    • Duke: it will take ages until we comply to this rule; however, it should be the goal.
    • geever: acceptable
  • 51
    • bayo: i dislike: what's going on int* x, y, z; Is y a pointer to int? anyway i also dislike multi declaration, i prefer
int *x;
int *y;
int *z;
    • Duke: if and only if we have an additional rule that requires 'one declaration per line', I'd prefer
int* x;
int* y;
int* z;
    • geever: I prefer
      int *x;
      and we should have a one declaration per line rule. We have it now.
  • 58
    • bayo: sorry i love break and continue
    • Duke: me too
    • geever: me too
  • 60
    • geever: I remember a rule or recommendation to use
      for (;;) {}
      instead. If I'm right, we have already changed some of our loops to that..
  • 61
    • geever: I think it actually makes understanding harder as you need to track more variables in head while debugging.
  • 62
    • Duke: as the handling of the exception is usually much shorter, it should be handled first
      • I think it is not a strong rule anyway. Bayo 10:26, 21 April 2012 (SAST)
    • geever: drop this rule
  • 64
    • geever: I like shorter code, however splitting helps debugging, so I can accept it
  • 70
    • geever: We have NULL almost everywhere, and it tells me it's a pointer, so I would rather use NULL than 0.
      • mattn: NULL is not valid in the c++ context in every case. foo(NULL) calls foo(int). which is not always what you want.
  • 71
    • bayo: should be what we have now TAB=4 (it is what i use here)
    • Duke: Yes. TAB=4. As we have dropped the column width of 80, we can afford that.
    • geever: one tab (the tabsize is doesn't really matter as we're not converting them to spaces)
  • 72
    • geever: first example as we have them now
  • 75
    • bayo: i prefer } else {
    • Duke: i prefer else {, but don't care much
      • Yes, both are readable for me Bayo 14:21, 21 April 2012 (SAST)
    • geever: I vote for } else {.
  • 77
    • geever: gcc suggests for (init; condition; update) {} and feel it better. With a lone semicolon I have a feeling that someone accidentally cut&pasted the call, instead of copy&pasting.
  • 80
    • geever: our current rule suggest writing cases on the same level as the switch itself.
  • 82
    • bayo: i prefer not to use that
    • Duke: I like it, coz it saves a line with just a '}'
      • but you dislike } else { cause it save a line... Anyway my point is "put a block everywhere cause it is safer", especially when you hack the code. Bayo 10:26, 21 April 2012 (SAST)
        • No, I like "else {" because the if aligns with the else. The 'is safer'-argument is strong. And I can easily live with it.--Duke 23:02, 23 April 2012 (SAST)
    • geever: I code like this ( if possible, not in Perl ;( )
  • 83
    • bayo: i prefer not
    • Duke: I hate it
    • geever: I hate it too
  • 84
    • bayo: but case 100:
    • Duke: agree with bayo
    • geever: agree, use our current rule
  • 85
    • bayo: i dont see why
    • Duke: Dislike it. Over-complicated. Foo(bar) is fine for me.
    • geever: agree with you: Foo(bar)
  • 87, 88, 89...
    • bayo: ugly!
    • Duke: beautiful !
      • Then, else the space, you agree with an if in 1 line, or a case in 1 line? Bayo 10:26, 21 April 2012 (SAST)
        • I general, I don't. But if it's a *series* of very similar ifs, resembling a table, it can increase readability a lot. --Duke 21:55, 23 April 2012 (SAST)
    • geever: it's horrible. It also hurts our current whitespace rules.
  • 92
    • bayo: but still use /** */ for method, class... bref, for the javadoc
    • Duke:
    • geever: we should use the C-ish ones only (and doxygen notation), we can still disable code with the good old #if 0
/* one lined */
/*
 * Some important
 * multiline
 */

Other things to avoid

The google styleguide has an interesting section "Other C++ features" about things NOT to use that should be discussed. http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml

Struct

typedef struct menuNode_s {
   int blah; /**< This is a documentation comment. */    
   int a; /* armour */ /**< Calling this 'a' then putting a comment immediately after is just dumb. Don't do it; use meaningful names */
   int blahBlah; /**< The total number of blahs. */
} menuNode_t;

Can we avoid thing like int a in structs. The code looks often hard to understand. Here we can use armour. I especially thing about the inventory code. Bayo 11:21, 3 September 2010 (UTC)