Site Navaid

hosted on

Coding Guidelines

When submitting patches, please follow all the guidelines outlined here. Submit the patch in the correct format, following the patch creation checklist below.

The Tracker maintainers decide whether to accept a patch. They are very strict about rejecting patches that do not follow all the formal procedures outlined here. Please help maintainers submit your contributions by following the guidelines very closely.

The maintainers of OpenTracker.SourceForge.net reserve the right to accept or reject any submitted patch based on their judgment.

Correctness and completeness

When submitting a patch, make sure it is well tested. This may sound obvious but patches with bugs will not be accepted. Use MALLOC_DEBUG, check for leaks, deadlocks, come up with creative ways of making sure your change didn't break some other behavior.

If you implement a new feature, make sure it is fully functional, complete and without unfinished holes that are exposed to the user. A cool new feature that may seem super exciting to you may be irritating to some other user if it contains parts that do not work well yet. Imagine you are contributing your change on the last day of an engineering cycle, right before a code freeze. That way users can download a new binary of Tracker after your change gets submitted and expect it to behave as a shipping-quality product.

Submit a separate patch for each feature, bugfix, etc. Unless two features/bugfixes depend on each other or are closely related, do not include them in a single patch.

Style guidelines

This document is not complete. If a guideline for a particular format is missing, follow the style of existing code (prefer using Tracker sources as a reference for this). If you have suggestions for things that should be clarified better, etc. please write us at axeld@users.sourceforge.net. Please don't send us suggestions of the kind "I like this indenting style better, could we switch?".

The Deskbar code didn't get cleaned up to follow these guidelines in some places, due mainly to time constraints. Help cleaning up the code is very welcome.

Rather than advocating a given coding style, the main goal here is code consistency - after a stream of external contributions and patches we would like the Tracker and Deskbar code base to remain clean, uniform, easy to read and maintain.

General

Make your code not stick out -- make it consistent with the rest of the code you are contributing to. You will see this rule stressed over and over throughout the guidelines and when submitting patches.

Indenting and white space

use tabs to indent functions. Set your editor to 4 spaces per tab.
the Tracker indenting style is close to the K&R style

First, let's use some examples to illustrate the main formatting conventions:

class Foo : public Bar {
	public:
		Foo(int32);
		virtual ~Foo();

		virtual void SomeFunction(SomeType *);
			// you may omit argument names if they don't help
			// documenting their purpose

		virtual const char *FunctionWithLotsOfArguments(const char *name,
			const char *path, const char *user);
			// indent long argument lists by a tab

	private:
		int32 fMember1;
		int32 fMember2;
		const char *fMember3;
};


void
Foo::Foo(int32 param)
	:	Bar(2 * param),
		fMember1(param),
		fMember2(param - 1),
		fMember3(NULL)
{
	...
}


template<class T>
const T *
Foo<T>::Bar(const char *bar1, T bar2)
{
	...
}

if (someCondition) {
	DoSomething();
	DoSomethingElse();
} else if (someOtherCondition) {
	DoSomethingElse();
	DoSomething();
}

if (someVeryVeryLongConditionThatBarelyFitsOnALine
	&& someOtherCondition) {
	// && operator on the beginning of the next line,
	// indented by a tab
	...
}

if (someVeryVeryLongConditionThatBarelyFitsOnALine
	&& (someVeryLongNestedConditionPart1
		|| someVeryLongNestedConditionPart2)
	&& lastPartOfSomeVeryVeryLongCondition) {
	
	// operator || in the nested condition is indented
	// by a tab
	...
}

for (int32 index = 0; index < count; index++) {
	DoSomething(index);
	DoSomethingElse(index);
}

// omit braces for single line statements, place statement on a new line

if (condition)
	DoOneThingOnly(index);

for (int32 index = 0; index < count; index++) 
	DoOneThingOnly(index);

// switch statement formatting

switch (condition) {
	case label1:
		DoSomething();
		break;
		
	case label2:
	{
		// need extra curlies here because of count
		// declaration
		int32 count;
		...
		DoSomething();
		break;
	}
}

...
	CallingSomeFunction(firstArgument * 2 + someMoreStuff,
		secondArgument, thirdArgument);
		// indent long argument lists by a tab
...

	const rgb_color kNeonBlue = {10, 10, 50, 255};

Misc. formatting

Try to wrap your code to a reasonable width of 80 - 90 columns.
Put exactly two newlines between functions.
Include a newline at every file end.

Identifiers

Use self-describing well chosen identifiers whenever possible. Avoid identifiers such as r (hard to search for), aMessage, theView, MyDraw (who's draw?). Avoid identifier pairs such as ProcessMessage and DoProcessMessage, AddTasks and AddTasks1. Use names such as rect, message, invokeMessage, view, targetView, DrawBorder, ProcessMessage and ProcessMessageInternals or ProcessMessageDetails, etc.
Classes, structs, type names, namespaces and function names start with uppercase letters and use InterCapsFormatting (no underlines).
variables start with lowercase letters and use interCapsFormatting.
member variables start with f like so:
```
	int32 fImAMemberVariable;
```
constants start with a k like so:
```
	const uint32 kOpenFile = 'open';
```
(note that this is different from the standard Be API constant names)

Variable declarations

declare variables as local in scope as possible, avoid declaring all variables at the top of a function like you have to do in C. The advantages here are it is easier to make sure variables are properly initialized and small code snippets are easier to copy-paste around.
Use descriptive names, avoid reusing a single temporary variable for different purposes.
Use full names such as "message" over abbreviations such as "msg". Use "rect", "frame", "bounds" over "r", "menuItem" over "mi".

Use BeOS-defined APIs, types, etc.

Prefer using a BeOS API utility call over "rolling your own".
Prefer using BObjectList over BList. BObjectList provides type-safety, optional item ownership and is designed such that additional template instantiations will not grow the code considerably.
Prefer BString over malloc, strdup, free, etc. for string operations.
Prefer BString << operators over fixed size buffers and sprintf.
Prefer using types defined in SupportDefs.h such as int32, uint32, over int, long, etc. Use status_t over int, int32 where errors are returned. Use off_t over int64 where appropriate.

Comments

Comment your code properly.
Prefer C++ style comments.

Comment properly but not excessively. (some examples of excessive commenting:)

	...
	index++;	// increment the index
	...
	

	...
	/* InitProgress
	 *
	 */
	void
	InitProgress(int param1)
	{
	...

Prefer commenting over and under the commented line (over when a comment relates to a chunk of code, under and indented by a tab when it relates to one specific line.

	// the trash window needs to display a union of all the
	// trash folders from all the mounted volumes
	// (comment about a block of code)
	BVolumeRoster volRoster;
	volRoster.Rewind();
	BVolume volume;
	while (volRoster.GetNextVolume(&volume) == B_OK) {
		if (!volume.IsPersistent())
			continue;
	...


	...
	BPoseView::WatchNewNode(&itemNode,watchMask,lock.Target());
		// have to node monitor ahead of time because
		// Model will cache up the file type and preferred
		// app (comment about the above line)

Avoid comments on the same line that create long lines (prefer comments on a separate line in general

	...
	if (this < is && a < very && long != condition) { // ...
	...
	if (this < is && a < very && long != condition) {
		// this comment is about the long condition
		// above and is much easier to read!
	...

Do not annotate your comments with your name or initials -- when your patch gets submitted, CVS will have your name in the checkin log. Anyone can identify your code that way.
Avoid expressing your sentiments in comments, do not include comments like this:
```
	// this is a hack!
```
Instead explain why you consider the respective code a hack:
```
	// the following code is fragile because it doesn't
	// handle overflows properly
```

Dead code and Debugging code

Do not leave dead, commented or #if 0'ed code behind just because you are not sure about your contribution. Your change should be top quality to begin with, improving the code you are replacing. Should there be a reason to back your change out or used for a reference, this can be done using the source control tools.
Do not leave simple commented out debugging printfs behind. These days bdb does a great job eliminating the need for most debugging printfs. Use ASSERTs for debugging purposes. Include debugging code in #if DEBUG blocks. Make sure debugging code compiles (make sure your change doesn't break existing debugging code, if it does, fix the debugging code as a part of the change) without warnings and stays correct. Use all the other tools from Debug.h to your advantage.

Miscellaneous

Use new-style casts (dynamic_cast, static_cast, const_cast, reinterpret_cast) over old-style.
Use const properly.
Prefer stack-allocated objects over heap-allocated ones whenever possible.
Use AutoLock, etc. for all locking and other resource acquisition, don't use Lock() and Unlock() on a BLocker directly. Don't use BAutolock, use the AutoLock template instead.

Don't assign in if, while statements:

	if ((err = entry.GetRef(&ref)) == B_OK)
		...

Avoid using assignments in while loops, don't use:

	BMenuItem *item;
	int32 index = 0;
	while ((item = ItemAt(index++)) != NULL) {
		...

Instead use wordier but as efficient for:

	for (int32 index = 0; ; index++) {
		BMenuItem *item = ItemAt(index);
		if (!item)
			break;
		...

Don't check for NULL before delete and free:

	// wrong
	if (fIcon)
		delete fIcon;
	
	// wrong
	if (fIconBuffer)
		free(fIconBuffer);

Don't put parentheses around return values:

	// wrong
	return (fList.ItemAt(index));

Don't put redundant parentheses in if statements:
```
	// wrong
	if ((a == 3) && (b != 4))
		...
```
Use inlines sparingly.
Use constructor initializer lists over initializing members inside the constructor body
Use built-in false/true instead of FALSE/TRUE #defines.
Alphabetize #include statements, group "includes" and <includes> separately.
Don't use <pathname/include.h> (<sys/stat.h> is an exception).
Use NULL instead of 0 for pointers.
Avoid gotos.
Don't use constructor call syntax for initializing pointers, etc. to NULL like this:
```
	BView *view(NULL);
```
Use the more traditional assignment:
```
	BView *view = NULL;
```
(Don't confuse this with the appropriate use of constructor calls for stack based objects and objects allocated with new). When comparing a function call result/variable with a constant, don't place the constant on the left side of the comparison like this:
```
	if (B_OK == file.InitCheck()) // don't use this style
		...
```
Programmers use this to make sure they do not end up assigning instead of comparing. The notation is a bit unusual though, placing the more important function call/variable to the less prominent position on the right. OpenTracker does not use this notation, a mistaken assignment will get caught by a warning.

Warnings

Submit code that triggers none of the warnings, enabled in the makefile, either on PPC or x86. There are no exceptions to this. The set of warnings was carefully chosen to give you the best possible set of checks by the compiler. Make sure you use both gcc and mwcc to check the warnings, check both DEBUG and non-DEBUG mode. Note that gcc and mwcc each trigger a somewhat different set of warnings for the same code. Humor both compilers.

Testing

Take great care in testing your code. Be creative at coming up with ways to test your changes. Consider sending your changes to a fellow developer for testing. Make sure your change works well on both x86 and PPC. The goal is to keep Tracker source tree in shipping quality at all times.

Check your code for memory thrashing bugs, try it with and without MALLOC_DEBUG.

Check your code for leaks. (you may uncover existing leaks in the code, contributions to fix them are welcome, send them in in a separate patch).

Submitting a patch - The patch creation checklist

Make sure your changes follow the coding guidelines above. As you have no doubt figured out by reading all the way to here, we are very strict about this.
Make sure you are using the current versions of the sources so that the resulting diffs are comparing your changes with the head of the source tree.
Submit a separate patch for each feature or bugfix. That way individual patches can be easily backed out should there be a problem.
Create your patch either by using:

cvs diff -u

(if you are using CVS)
or

diff -u original-file changed-file

(if you are using a source archive - you can also create differences for the whole directory contents using diff -r)
In the latter case include the old code first, the new code last -- in the patch anything you added will be prefixed with a +.
Remove all/any lines that reference files without changes.
In the email body or the SourceForge Patch tracker include a short, descriptive paragraph suitable for the source control log, describing your change, including bug numbers, etc. This paragraph will be pasted in the CVS checkin log for all the world to see and get informed about your patch. Pay as much attention to this short paragraph as you are to writing the actual code -- it's a summary of the hard work you have done and you want to explain it well to everyone else.
In addition to the descriptive paragraph described above, include any other comments, explanation, etc. that will make it easier for the maintainers to understand and evaluate your patch. Describe how you tested your changes.
Include a list of all the files you modified on a single line, separated by spaces:
tracker/PoseView.cpp tracker/Tracker.h
Send the patch file as an attachment in your email. Do not paste the patch directly into the email body. The patch may get get garbled if you do.
If you are using the SourceForge patch tracker, use the upload feature to submit your diffs - note, that this doesn't work with NetPositive.
Send your patch email to axeld@users.sourceforge.net, or use the Patch tracker from SourceForge. Include a title that represents your patch nicely such as: "IconCache leak fix."
Maintainers will often reply in response to your patch, pointing out things to fix up, etc. before a patch can be checked in. Please always follow the maintainer suggestions closely and respond by sending a new corrected patch. Please do not expect the maintainers to rework your changes, you want to be able to claim all the credit for your great patches!

Please help us by strictly following all the steps of this formal process.

Books

The following fine books can be used as a reference for desirable C++ coding guidelines.

Stanley Lippman, C++ Primer, 3rd Edition
Bjarne Stroustrup, The C++ Programming Language, 3rd Edition
Scott Meyers, Effective C++, 2nd Edition
Scott Meyers, More Effective C++