A Poor Man Coding Style Guide for Rhaptos
Document Actions
Submitted by
nmueller.
on 2006-08-21 16:27.
Here I want to provoke a discussion regarding coding styles in Rhaptos. E.g. (1) Shall we use TABs rather than Spaces for indentation (2) How should a proper function/method should look like regarding documentation (3) What about error handling? Should any if-statement be complete, i.e., should any if-statement comprise an else-statement (4) What about code line wrapping for a convenient reading etc...
Just feel free to leave any comment here regarding my upcoming poor man's coding style guide for Rahptos.
Note 08/24/06
Thanks all for their great and enthusiastic contributions!
Just to let you know: This weekend I will read all comments on this issue and generate a survey of the current discussion state so that we get back to some structure (otherwise at least myself will lose track ;-). Furthermore I also hope the re-structuring will help us to make some (final) decisions on some discussion items.
You all have a nice weekend!
(1) Spaces, and four of them, unless the existing code does otherwise.
(2) Docstring with purpose, warnings, and, unless blindingly obvious, parameter and return values. Tricky parts of the code should also be commented. This includes notes for future improvements and pitfalls, anything that required research, places where order matters but it might not be obvious, and anything that looks like cruft but is there for a good reason. Hopefully methods will not be long enough that serious algorithmic comments are necessary, but if they are, the comments should describe the flow of the code.
(3) Generally we will want to check conditions before doing something rather than catch and handle exceptions. (This is the "ask permission, not forgiveness" approach, and is due to performance differences between the two ways.) If statements need not be complete, and rarely are.
(4) code should be wrapped, where convienent at 80 characters, but given modern editing environements, up to 100 characters is fine. Code clarity should not be sacrificed to line-length concerns.
Yaaay! I'm so strongly in favor of this that I won't even complain much about doing four-spaces instead of two-spaces. ;)
(a) Where can I find the Plone coding guidelines?
(b) Regarding "[...] This is not consistent across existing code, but such inconsistencies should not be fixed[...]" and (1): Why not also modify existing code? We could do that incrementally after one has "visit" such an existing file.
(d) Regarding (2): "Docstring with purpose, warnings, and, unless blindingly obvious, parameter and return values."
I would prefer to strike out "unless blindingly obvious" here.
Each method/function should be fully documented, i.e., each docstring of a method/function should look like the following
def foo(param1, param2):
"""This is the purpose of the function foo
param1:type This is the description of param1
param2:type This is the description of param2
return This is the description of the return
value and type of foo
throws: This is the description of each possible
exception thrown by foo
remarks: This is where any other information
related to the current function is
placed, e.g. warning, hints, usage,
preconditions, invariants etc. """
(e) Regarding (3): Is it always possible to check
conditions in advance? Wouldn't that be a kind of
verification? I would suggest to force all developers
to handle exceptions, e.g. by guiding "kernel developers"
to throw exceptions in their "raw" methods/functions. Furthermore "If statements need not be complete,
and rarely are." is not a proper statement, sorry, but
I suggest each condition should have a counterpart even if
it is just an "pass". Besides code robustness, that also
helps new developers to understand the code quicker.
(f) Well, actually I don't care what the line length will be, but --- like in all previous cases --- I prefer to keep it consistent in the whole project.
> Why not also modify existing code? We could
> do that incrementally after one has "visit"
> such an existing file.
My instinct is because we'd then have an SVN discontinuity in almost every one of our files. Reflowing an entire file makes it nearly impossible to determine what the actual changes were in that iteration. Even if that particular iteration is well-documented in the commit log, reflowing also makes diffing against historical versions difficult.
I could also be completely wrong here. Cameron? :)
(b) Addressed separately. I will note, though, that while I don't think number-of-space problems should be fixed, tabs mixed in with spaces should be fixed. That's a bug (even technically illegal) and not a style preference.
(d) That's a very good comment style, and highly encouraged. But since none of it's machine readable, the only absolute requirement (as in, I will complain) is that all that information be clearly provided. The comment should also match the method: the longer and more complex the method, the more formal the docstring should be. I have no problem with::
def reverseWithCap(str):
"""Given a string, return that string backwards, but with the new first character capitalized."""
return ...
But that would (probably) not be an acceptable docstring for 'convertPage' or something. There are, of course, many violations of this in the existing code, which should be fixed where possible.
(e) No, it's certainly not possible to check everything in advance. File and network operations, for example, can only report problems with exceptions. But when, for example, getting an attribute that may or may not be on an object, use hasattr instead of 'except AttributeError'. Same for types.
I do not know of a good reason to require else clauses, especially to the point of 'else: pass'. I have never seen this done, and have never felt a need for it. Feel free to explain, though.
(f) Being dogmatic about line lengths will inevitably lead to contortions of the code. In other languages, it is much easier to be strict about line lengths, but since in Python newlines matter syntactically, ending lines early for space concerns is not a natural thing to do. Lines should be within 80 characters, unless this impacts code clarity.
Even so, I'm not likely to notice until about 100 characters.
I really mean, I don't think they should be fixed *for their own sake*. If you're heavily refactoring something, and it's going to change everything anyway, go ahead and fix the spaces. It's sort of like building codes: you don't have to fix existing conditions unless you're well into it.