[Insight-developers] Re: Review Directory : Coding Style Problems

[Luis Ibanez]

Hi Gaetan,

The line length is an important part of the coding style.
It makes possible to have side by side comparisons between
different versions of the files.

It also plays an important role in increasing the power
of CVS comparisons, which are all line-based.

If all the code of a C++ class was written in a single
line, then CVS will rapidly become useless, because all
the time will simply be reporting that the single line
changed.

Short lines are also useful for debugging, since debuggers
will manage break points based on individual lines of code.


The line length, and in general, coding style is not for the
computer, nor the compiler. It is a tool for the software
maintainers. Code is intended to be read by humans.


The Insight Consortium spent a lot of time in the early days
of the projects defining the coding style. It is too late for
changing the coding style of 200,000 lines of code.

We have *enough* difficulties enforcing the current style in
the 30 or 40 files in the Code/Review directory.

Please *use KWStyle* before committing changes to ITK files.

You may also find useful to look at the code review
check list in the following Wiki page:

   http://www.itk.org/Wiki/ITK_Code_Review_Check_List


Respecting the coding style is one of the implicit rules
that you accepted to respect when your received CVS write
access.


More than 80% of the time that we have dedicated for adopting
the code from the Insight Journal has been spent in fixing
coding style issues. It will be interesting if we could rather
spent some of that time in bringing more new classes into the
toolkit.



    Regards,


        Luis



========================
Gaetan Lehmann wrote:
> 
> Hi Luis,
> 
> Sorry for that, I just didn't thought I was doing something wrong.
> I tried to fix some errors pointed by KWStyle, and I must say that's a  
> painful task.
> The worst in that, is the feeling that most of the fixes are useless, 
> and  only a minority of them have a real interest: the line length is 
> the most  frequent error, and most of the time, fixing that make the 
> code more  obfuscated.
> 
> The line length limit was useful some time ago, but is now quite 
> useless.  All the computers are able to display more than 80 characters, 
> and all the  compilers are able to read more than 80 characters. With my 
> text editor,  112 chars are displayed if I keep the navigation tree on 
> the left of the  window, and 147 without it.
> 
> For code clarity, and to avoid a boring and (most of the time) useless  
> task to the developers, it would be nice if the line length limit can 
> be  removed, or at least increased by 20 or 30 characters.
> 
> Gaetan
> 
> 
> 
> On Mon, 22 Jan 2007 17:38:06 +0100, Luis Ibanez 
> <luis.ibanez at kitware.com>  wrote:
> 
>>
>> Hi Gaetan,
>>
>> Your recent changes to the Code/Review directory
>> are breaking the code style checks for several
>> files.
>>
>> We are spending a large amount of time fixing
>> coding style issues, and that time is wasted
>> when other developers make changes without
>> respecting the coding style.
>>
>>
>> ---
>>
>>
>> Before you commit changes in this directory
>> you may want to run KWStyle as:
>>
>>
>>    KWStyle -xml ITK.kws.xml  -v    filename
>>
>>
>> in order to verify that the file "filename" is
>> satisfying the required coding style.
>>
>>
>> You can download KWStyle from :
>>
>> http://public.kitware.com/KWStyle/download.htm
>>
>>
>> The ITK.kws.xml file is generated by CMake
>> in the binary directory where you are building
>> ITK.
>>
>> Note that you should enable
>>
>>            ITK_USE_KWSTYLE
>>
>> In your CMake configuration of ITK.
>>
>>
>> Please let us know if you find any problem
>> building or running KWStyle.
>>
>>
>>
>>    Thanks
>>
>>
>>      Luis
>>
> 
> 
>