[Clam-devel] style

[David García Garzón]

On Monday 25 May 2009 18:29:40 Dirk Griffioen wrote:
> Hi All,
>
> I know there is a coding guideline and discussing style is very
> dangerous :), but if I may, I would like to share some thoughts on what
> I read; the following line is from the OfflineNetworkPlayer.cxx file.
>
>
> for(std::vector<SndfileHandle*>::iterator it=infiles.begin();it!
> =infiles.end();it++) delete *it;

I would have typed:

// somewhere in the class or in the same function if needed:
typedef std::vector<SndfileHandle*> SndfileHandles; 

// then
for (SndfileHandles::iterator it=infiles.begin(); it!=infiles.end(); it++)
	delete *it;
// Note that there are spaces.

Or, being a vector, i lately use directly indexes:

const unsigned nHandles  = infiles.size();
for(unsinged i=0; i<nHandles; i++)
	delete infiles[i];

Often i don't cache the size if the process is not a bottle neck. This 
destructor is not a bottle neck.

> - please dont type everything as if it was one big word, use proper
> spacing like when writing text: source is meant to be read by humans
> (computers dont read)
>
> - please type ++i instead of i++:  http://www.parashift.com/c
> ++-faq-lite/operator-overloading.html#faq-13.15

If you need such fine optimization we just don't use iterators but indexes.

> - please dont put multiple statements on 1 line (return or delete after
> if for instance): debuggers cant set breakpoints

Fully aggree although i dont always follow that. (I am very hypocrital 
regarding the coding style ;-) )

> - use typedefs, its shorter, more clear as it expresses what you're
> doing and localizes the 'type', when you change it, you only need to
> change one place
>
>     for(SndFileHandles::iterator it = infiles.begin(); it !=
> infiles.end(); ++it)
>         {
>         delete *it;
>         }

Fully agree regarding the typedefs see above. The indentation of the {} above 
is not CLAM style compliant. Using claudators or not is free to the writer 
when we are talking of one liners.

> - please note: with the use of smartpointers, this whole line becomes
> obsolete as they and their resources are destructed when the vector is

I know the implications on what we are doing now, we have a clear memory 
management policy, following it is not difficult and spoting the bugs of this 
kind of construction is very straight forward for us. I cannot say the same 
of the use of smart pointers as most of the things happen behind the scene.

> - please do not indent in namespaces: c++ is not java or c#

For me this is a matter of taste. We can find both styles in sources, but i 
can see how either option can affect readability. Luckily in clam we use tabs 
for indentation so if you indent namespaces it you can reduce the tab size 
and deep indentations are still readable. For new files I tend not to indent 
them but i don't use to change that unless i find mixed styles in one file.

If you do 'aesthetic' changes on a file please try to keep them separated from 
more critical commits so if we need to trace back bugs on the svn we can 
safely skip those ones on the diffs.

David.