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

[Gaetan Lehmann]

Luis,

I'm sure I'm not writting correctly in english, and that my words can be  
interpreted with another meaning that what I wanted to say. Sadely, I'm  
already doing my best, so please, when you read one of my mails, try to  
remember that I'm hidding nothing in the sentences, and that most of my  
mails are there to try to make things better.

On Thu, 25 Jan 2007 14:19:44 +0100, Luis Ibanez <luis.ibanez at kitware.com>  
wrote:

>
> 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.

I *never* asked to be allowed to write several statements on a single  
line, but rather not be asked to break a line because it is a little too  
long. Breaking a statement on several line often make it less clear than  
when it is on a single line, even this line is 100 characters long.

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

A good point !

>
> 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.
>

That's why I asked that, to avoid spending time making the code less  
readable

>
> 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.
>

I *never* asked to change the style of the 200000 lines of code ITK, and I  
*never* asked to change all the coding style of ITK.
I only asked to not be forced to fix the too long lines (according to ITK  
style).

> 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.
>

All the files I have committed since your mail are checked with KWStyle

> 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.
>

And that's what I'm doing, excepted for the line length - just forgot  
about this one, and already apologized for that

>
> 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.

In the files I have fixed this morning, at least 95% of the problems was  
the length of the lines. Like you, I would really prefer spending this  
time doing things more useful.

  - that's why I ask again if the line length limit can be increased, or  
removed

Gaetan


>
>
>
>     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
>>>
>>



-- 
Gaëtan Lehmann
Biologie du Développement et de la Reproduction
INRA de Jouy-en-Josas (France)
tel: +33 1 34 65 29 66    fax: 01 34 65 29 09
http://voxel.jouy.inra.fr