23-01-2004 17:35									#1
	PROJECT: Coding conventions and standards Enter the AD-FREE zone

It has been discussed before but not adequately, nor final decissions has been made on the issue in my opinion.
What coding style should we adopt, if any? Is it necessary? Should we follow some standards?

I beleive in some cases it could be critical.
Generally it would help a bit if we would put some fundamental rules.

a) ANSI compliant code. We should code exclusively under the ANSI standard unless absolutely necessary to go with Microsoft Visual C++ specific coding.

b) Comments. We can go with fromafar's style, perhaps add some additional information in the header of each file altered. I recommend we add comments at the start and the end of the new code we enter to mark the start and end of it, plus the name of the coder who added the code, and a small description if needed. Whether \\ or /* */ should be used or a combination is under discussion also.

c) Naming style. What we need here is a descriptive name for each variable, constant, object etc. Secondary styling like camel-notation, hungarian-notation, underscore character usage etc. can be discussed but I don't think they are important and each coder could co with his style, perhaps with a common agreement on the uppercase and lowercase letters.

d) Whitespace. How much whitespace should be used to sort the code and make it more readable? How much do we indent?

e) Braces. Can the way braces are put cause problems?

f) Long lines. Should we keep long lines or break them down into more?

Also other areas such as access, class definitions, include files, assertions, libraries, etc. can be discussed here whether it is early or not and will be of more use when we start to make extensive changes and additions to the code.

	24-01-2004 01:25									#2
	Re: PROJECT: Coding conventions and standards Suffering from ads?

quote: Originally posted by Keygen b) Comments. We can go with fromafar's style, perhaps add some additional information in the header of each file altered. I recommend we add comments at the start and the end of the new code we enter to mark the start and end of it, plus the name of the coder who added the code, and a small description if needed. Whether \\ or /* */ should be used or a combination is under discussion also.

Actual it is not necessary to add comments at the end of each new piece of code as the #endif clearly states it. Maybe we could put a //ACTIVISION_ORIGINAL behind it, but actual it only does make sense if the new piece of code is very long, so that you can't see the start and end of the new code on one glance.

-Martin

	24-01-2004 03:26									#3
	Re: PROJECT: Coding conventions and standards Help yourself to an AD-FREE life

Hi,

quote: Originally posted by Keygen a) ANSI compliant code. We should code exclusively under the ANSI standard unless absolutely necessary to go with Microsoft Visual C++ specific coding.

Yes, this would be wonderful (especially for porting)... ;-)

quote: Originally posted by Keygen b) Comments. We can go with fromafar's style, perhaps add some additional information in the header of each file altered. I recommend we add comments at the start and the end of the new code we enter to mark the start and end of it, plus the name of the coder who added the code, and a small description if needed. Whether \\ or /* */ should be used or a combination is under discussion also.

I think comments should be used to document how non-trivial code works, but not be used for each code change. Guess something like this:

code:

// Coder: blafasel
// Fixed case of header
#include "C3Something.h"
// EOF: Coder blafasel
// Coder: bar
// My implementation
#include "IBar.h"
// EOF: bar

// Coder: bar 
// Added Bar::foo()
void
Bar::foo()
{
  // ... (code by bar)

  // Coder blafasel
  // fixed error handling of code

  // ... ( some code by bar)

  // EOF Coder blafasel
}
// EOF Coder Bar::foo()

This is a simple example. What about nested changes (by different coders), etc? You don't see the code after a while, but mainly a bunch of comments with some code inbetween.
If that information is needed, it would be better if a central version control system like cvs would be officially used, so each coder could enter that information on commit. When reviewing code changes, you could have a look at the logs and do a cvs diff between the wanted revisions. As each coder had his own login, the user name in the log would be the "who changed" info.
We could stick files to a simple header containing a contributed line where everybody wanting to do so may add his credits.

quote: Originally posted by Keygen c) Naming style. What we need here is a descriptive name for each variable, constant, object etc. Secondary styling like camel-notation, hungarian-notation, underscore character usage etc. can be discussed but I don't think they are important and each coder could co with his style, perhaps with a common agreement on the uppercase and lowercase letters.

I think variables should be capsulated and therefore they are hidden for the holder of an objects instance. I'd say for access there should be getters/setters (get/set prefix), with capitaled short descriptive what the getter/setter returns/updates.

quote: Originally posted by Keygen d) Whitespace. How much whitespace should be used to sort the code and make it more readable? How much do we indent?

I'd say the best would be space identation of say 2 or 3 spaces per indent level. Space indentation gives a similar look regardless what editor you choose, thus everybody sees the same structured code on his screen. Any editor i know is configurable using space indentation instead of inserting tabs.

One thing to add: Line delimiters should only be newline (\n), and not carriage return (\r) additionally. I.e. before posting, use dos2unix on that file.

quote: Originally posted by Keygen e) Braces. Can the way braces are put cause problems?

I use a mixed style for that. In short code, they all go in one line.
Within long expressions, i also put the closing ")" indented under the corresponding "(". {} pairs always have same indentation. It's just for readability, you could put them where you want...

quote: Originally posted by Keygen Also other areas such as access, class definitions, include files, assertions, libraries, etc. can be discussed here whether it is early or not and will be of more use when we start to make extensive changes and additions to the code.

I often use .hpp extension for include files, so *.[ch]pp are c++ source files in shell and for tools like find.
There are some recommendations on
C++ Recommendations
Mozilla C++-Portability guide

Oh, and prefer the last one over the first one.


Ciao
Holger

	24-01-2004 17:53									#4
	Enter the AD-FREE zone

A CVS will indeed be needed so that code files changed by a dozen coders don't start to have 100 kb worth of comments.

The hard part to agree upon might be bracing style. I have a habit of putting the opening { on the same line as the if or for statement. Also, I will always surround the conditional code (after an if statement) with a pair of braces, even if there's only one line of code, where the braces can be skipped.

	25-01-2004 23:42									#5
	Increase the size of your Attachments

Hi Solver,

quote: Originally posted by Solver The hard part to agree upon might be bracing style. I have a habit of putting the opening { on the same line as the if or for statement. Also, I will always surround the conditional code (after an if statement) with a pair of braces, even if there's only one line of code, where the braces can be skipped.

Well, there indeed many bracing styles, and i prefer yours.

The style most uncomfortable for me is the gnuish style (many control statements and you have to scroll, scroll, scroll and you don't know what was in the beginning, so you scroll, scroll and scroll...).

Ciao
Holger

	01-06-2004 20:28									#6
	Support Apolyton, pre-order Civilization IV

So, the issue of documenting the code has been brought up, and I think it is fairly important to have the code documented well (and I mean more of documenting the API of different modules even than simply explaining tricky pieces of code inline, though that would be nice too).

I suggest using some commenting style that is understood by doxygen, because it is the best tool to generate HTML documentation for the code that I know of for C++. That being said, it does take a loooong time to generate all docs for the CtP2 project. I just ran doxygen yesterday, and it took over 20 minutes, I think, and the resulting documentation is almost 300MB, and remember that is without *any* comments (I ran the doc generator on the original sources).

So, I suggest using one of doxygen's commenting styles: either javadoc style:

code:

/**
 * Description of func.
 *
 * @param p Description of param.
 * @returns Description of return value.
 * @author Who last messed with it.
 * @date When was it last messed with.
 */
int func(char* p) { ... }

code:

///
/// Description goes here
///
int func(char* p) { ... }

code:

/////////////////////////////////////////////////
/// Description goes here
/////////////////////////////////////////////////
int func(char* p) { ... }

code:

//!
//! Description goes here
//!
int func(char* p) { ... }

And others. So I'd say - use whichever one you want - as long as doxygen can understand it.

More on doxygen commenting standards here:

http://www.stack.nl/~dimitri/doxygen/docblocks.html

-----

Now, there is currently no particularly organized documentation effort, and I don't know that it is terribly needed - a lot of code will probably be never messed with any way - though I think it would be a nice thing to do, in case someone wanted to do some major changes.

So, what I suggest is that when people fix bugs - document stuff that you mess with. What I mean is: you figure out what's going on any way - share that knowledge. Suppose you change function foo in class Bar in file Bar.cpp. Then document function foo. Say function foo does such and such, and put in a revision history saying on date such and such, I, such and such, made changes this and that, so that such and such works properly now. Maybe if class Bar isn't that big, even document the whole thing.

That way, while we are not exactly spending effort exclusively documenting the code, documentation is still being done. It is better than nothing.

Just a suggestion.

	01-06-2004 23:26									#7
	Remove this text

quote: Originally posted by vovan So, I suggest using one of doxygen's commenting styles: either javadoc style: code: /** * Description of func. * * @param p Description of param. * @returns Description of return value. * @author Who last messed with it. * @date When was it last messed with. */ int func(char* p) { ... } I personally, prefer this documentation style, but that's just me.

I dislike this style, because it prevents commenting out large blocks of code using /* ... */, but perhaps that's just me .

Personally, I'd probably use ///, since that's what C# needs for its documentation generation.

	02-06-2004 03:09									#8
	Put an end to popups!

quote: Originally posted by J Bytheway I dislike this style, because it prevents commenting out large blocks of code using /* ... */, but perhaps that's just me . Personally, I'd probably use ///, since that's what C# needs for its documentation generation.

Well in that case we should use ///, it is also less work to convert the existing comments, As we dont have to bother with the existing lines:

code:

//-------------------------

	02-06-2004 03:54									#9
	Lose 30 kilos (of popups)

quote: Originally posted by J Bytheway I dislike this style, because it prevents commenting out large blocks of code using /* ... */, but perhaps that's just me.

Preprocessor is your friend.

Do

#if 0
...large chunk of code...
#endif

instead of

/*
...large chunk of code...
*/

	02-06-2004 11:38									#10
	Remove this text

quote: Originally posted by vovan Preprocessor is your friend. Do #if 0 ...large chunk of code... #endif

I don't like that either because it doesn't get syntax-highlighted, and is easy to miss for others browsing the code .

	07-06-2004 03:39									#11
	Asserts Enter the AD-FREE zone

What is the opinion or standard for handling assert messages that will be raised (also) in valid situations?

IMO this should changed in cases wheer it is possible, so that only asserts will be declared that are completely valid. Because that leads to a debugging where only than a assert message pops up, when there is really a problem.

And I added the example, that brings me to this question. The comment was addded by me after searching for the reason I got the assert message.

By the way: If anyone has some suggestions what to do, to change this assert or the dependant code so that only in real wrong situations a assert message pops up, i'm really interested to hear.

If someone wants a savegame, that causes the assert by finishing the current turn, let me know to add one.

Attachment: slave driver attack.jpg
This has been downloaded 84 time(s).

	07-06-2004 04:10									#12
	Support Apolyton, buy Call to Power 2

It seems to me what should be happening is that contact should be initiated by the combat, before war is declared... Any circumstances leading to a declaration of war should certainly cause contact.

	07-06-2004 16:57									#13
	Increase Your PM Length

In this particular case, I would suggest weakening or removing the Assert, because this is a valid game situation, and the game does handle the situation well. If the slaver is invisible to the victim, no contact is established. The victim may be enraged about the captured citizens and would like to declare war, but can't, so actually doesn't.

You may still want to Assert the validity of player_ptr and foreigner_ptr, but the current Assert is not helping in finding bugs. It is merely irritating. Fortunately, it does not really occur many times, and all you have to do is press Ignore.

	08-06-2004 18:33									#14
	Get a bigger avatar today!

I thought this was the situation of a slaver attack, not a slave raid. In this case, the victim can see the slaver (when it attacks).

	10-06-2004 05:29									#15
	Support Apolyton, buy Galactic Civilizations: Deluxe Edition

We seem to agree, that a assert should only be set, if the condition of the assert is always true, as long the program works correct.

This leads to the need to change something about the handling of the assert in this special case with the slave driver.

More questions raise for me now with the ctp2 code:

1) What about unused functions, methods or classes? Shall they commented out or removed from the actual further developed code to reduce complexity?

2) I'm not sure about the ACTIVISION_ORIGINAL thing, that will be used now, to indicate the changes. As more and more changes are made to the code, this clutters and blows up the source and it is harder to look through the code and understand and/or change him.

It would be much better to have a kind of routine to make versions of the different source files. So you have always the prior versions of the file to see what's changed in time using e.g. WinDiff or another tool. And your actual source is clean and only includes actual needed stuff.

To compile and build the orignial you can use an archiv of the sourcefiles.

	08-07-2004 21:04									#16
	Increase Your PM Length

Hi,

quote: Originally posted by Lui2 1) What about unused functions, methods or classes? Shall they commented out or removed from the actual further developed code to reduce complexity?

Well, i suggest using both possibilities:
For dead apolyton code, i'd say it should be removed from the code.
Dead code of CTP2 Source Project should only be removed, if it gets archieved somehow and a poll for removal resulted in a majority vote for removal.

The reason is: apolyton code is always reproducable by the unmodified source package. However code/modifications made of the projects coders is not reproducable by the source package, and tends to vanish (e.g. look for the ALL packages released on March).
Unless there is no version control system (VCS) used, i suggest this, though i don't like dead code, either.

quote: Originally posted by Lui2 2) I'm not sure about the ACTIVISION_ORIGINAL thing, that will be used now, to indicate the changes. As more and more changes are made to the code, this clutters and blows up the source and it is harder to look through the code and understand and/or change him.

Well, thats also a major question regarding the linux port and merging the changes back into the main line, because these defines would be nested with other system programming stuff.
I posted a vote for it on #ifdef ACTIVISION_ORIGINAL for that.

quote: Originally posted by Lui2 It would be much better to have a kind of routine to make versions of the different source files.

Yes, i also see the need of an official vcs. I prefer subversion for certain improvements (atomic commits, efficient binary file handling, possibility to use it through proxies, apache2 modules -> use with ssl, file renames, just to name a few), though CVS has been named only so far because its mature and the most popular so far.

Ciao
Holger