On Wed, Jul 13, 2011 at 10:52 AM, Yehuda Sadeh Weinraub
<yehudasa@xxxxxxxxx> wrote:
> Following are my remarks to the new coding style document:
>
> On Wed, Jul 13, 2011 at 10:34 AM, Yehuda Sadeh Weinraub
> <yehudasa@xxxxxxxxx> wrote:
>> - for regular classes, CamelCaps, private: section, etc.
>>
>> * Naming > Variable Names:
>>
>> Google uses _ suffix for class members. We haven't up until now. Should we?
>
> No.
Quick quiz: line 144 of rgw/rgw_op.cc, is ret a local or class
variable? How about line 1340? Or line 1419?
I am on commit 4761317b754c6a903862d1913b53c8ab5e06b8eb .
It's just confusing for class variables to teleport in with the same
names used by local variables elsewhere in the file. If you are lucky,
you'll get a type mismatch, but what if you're not? The fact that
local variables shadow class variables is "obvious" to someone with 10
years of C++ experience, but not so much to people from the community
trying to read the code.
This is why languages like Python don't have the "implicit this"--
every access to a class variable in python must look like
self.variable.
We should follow Google's lead here.
>
>>
>> * Naming > Constant Names:
>>
>> Google uses kSomeThing for constants. We prefere SOME_THING.
>>
>> * Naming > Function Names:
>>
>> Google uses CamelCaps. We use_function_names_with_underscores().
>>
>> Accessors are the same, {get,set}_field().
>>
>> * Naming > Enumerator Names:
>>
>> Name them like constants, as above (SOME_THING).
>>
>> * Comments > File Comments:
>>
>> Don't sweat it, unless the license varies from that of the project (LGPL2) or
>> the code origin isn't reflected by the git history.
>>
>> * Formatting > Conditionals:
>>
>> - No spaces inside conditionals please, e.g.
>>
>> if (foo) { // okay
>>
>> if ( foo ) { // no
>>
>> - Always use newline following if:
>>
>> if (foo)
>> bar; // okay
>>
>> if (foo) bar; // no, usually hardler to visually parse
>>
>>
>>
>>
>> The following guidelines have not been followed in the legacy code,
>> but are worth mentioning and should be followed strictly for new code:
>>
>> * Header Files > Function Parameter Ordering:
>>
>> Inputs, then outputs.
>
> I generally agree, just that there's a class of strcpy like functions
> that put the destination first. This usually makes sense when you have
> multiple functions that do similar things with varied input
> parameters:
>
> char *strcpy(char *dest, const char *src);
>
> char *strncpy(char *dest, const char *src, size_t n);
>
> So in both cases, dest and src are the first and second parameters,
> which looks more consistent.
Well, nothing is set in stone. There are always exceptions if it
really makes sense. If you are designing a function patterned after
strcpy or whatever, making it "look like" strcpy probably makes a lot
of sense.
>
> And there's also the case of using default values, which usually
> (though not always) applies to the input parameters. Enforcing strict
> input before output ordering wouldn't work there.
Well, the Google document says not to use default parameters because
"they obscure the interface."
So that is a non-issue.
Check out commit 6e2b594b3329cb08bf54cd325776b236f2e6c4f1 for an
example of how default parameters can really bite.
Colin
>
>
>>
>> * Classes > Explicit Constructors:
>>
>> You should normally mark constructors explicit to avoid getting silent
>> type conversions.
>>
>
>
> Yehuda
> --
> To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
