Hello: I am working on modifying PETSc to interoperate with the MOAB mesh library. I'd love to get some of Karl's scripts to ensure that my code is compliant with the style guide. Are they available somewhere? Thank You Milad On Mon, Jan 21, 2013 at 10:19 AM, Sean Farley <[email protected]> wrote:
On Mon, Jan 21, 2013 at 10:06 AM, Karl Rupp <[email protected]> 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, 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.
Just my 2 cents…
Thanks, Karl. I appreciate your effort on this front.