[petsc-dev] I hate this one

[Matthew Knepley]

On Mon, Jan 21, 2013 at 10:06 AM, Karl Rupp <rupp at mcs.anl.gov> wrote:

> Hi Matt,
>
>
> >     Again, there are limits to everything, and this surpasses the useful
>
>>     limit to this kind of specification. This is not personal
>>     expression, this
>>     is ease of reading.
>>
>>
>> Also, judging by the ENORMOUS number of source code changes, "everyone"
>> was not following the informal guidelines.
>>
>
> Yes, the changes are 'enormous' if you count the absolute number of lines.
> However, my overall impression while refactoring was that I touched ~1% of
> the total code base only. It's the huge size of the code base that makes
> the number of changes appear to be high.
>
> Also, an estimated 50% of the violations stem from a copy&paste of older
> code or previous examples rather than intentionally ignoring the style
> guidelines. Thus, by bringing everything to a consistent state, a (large)
> number of such copy&paste-induced violations are no longer going to happen
> at all.
>
> The PETSc coding style guide needs to define exactly one style. Your style
> preference obviously differs. My personal preference differs slightly, too.
> Maybe Jed and/or others would prefer another slight variation.
> Nevertheless,  allowing a bit more flexibility in one's own personal coding
> style by following one common coding style is to the benefit of the whole
> project. Visual consistency is an obvious one,


Not obvious to me. Replied to this elsewhere.


> but most importantly it aids in setting up other scripts for increased
> uniformity among the different PETSc modules.
>
> You may argue that it does not matter whether we have 'for(...)' or 'for
> (...)'. While this is certainly right for a compiler, I would like to give
> you one example where it *does* make a difference:
>  src/mat/interface/matrix.c:
>   PetscErrorCode  MatSetValuesAdifor(Mat mat,PetscInt nl,void *v)
> So, keeping consistency with the style guide, we would write
>   for (...) {...}
>   MatSetValuesAdifor(...) {...}
> and there is the additional blank contributing to the difference between
> the function call and the for-loop. If, however, we have
>   for(...) {...}
>   MatSetValuesAdifor(...) {...}
> any scripts additionally need to check for characters in front of 'for'.
> Fortunately, it is *currently* sufficient to just consider all characters
> other than a blank in front of 'for' to be a function call. However, one
> day we might find code such as
>   do_something(...);for(...) {...};
> or even
>   cilk_for(...)
> and all of a sudden we are unable to use any simple checker scripts any
> more. With a common PETSc style, the additional blank would still allow for
> far simpler checks and refactoring if required.
>

Any transformation which relies on whitespace is way too fragile.

   Matt


> Just my 2 cents...
>
> Best regards,
> Karli
>
>


-- 
What most experimenters take for granted before they begin their
experiments is infinitely more interesting than any results to which their
experiments lead.
-- Norbert Wiener
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.mcs.anl.gov/pipermail/petsc-dev/attachments/20130121/0ff434f2/attachment.html>