Code style question: long function return types

[Jakob Kummerow]

Hi,

I've come across a style issue that I'd like your input on: functions
returning extremely long return types, so that there's no way to put
return type and function name on the same line without breaking the 80
char limit. The style guide doesn't cover this situation, it just says
"The return type is always on the same line as the function name.",
which obviously isn't applicable here.

Question: do we prefer to indent the second line (i.e. the function
name), or don't we? I've seen both in existing code.

Examples:
Note 1: These aren't my code, so please don't tell me to just rename
something to make it shorter.
Note 2: Observe that I formatted the second example with indentation,
the first without, to demonstrate the difference.

const ConfigurationPolicyPrefKeeper::PolicyToPreferenceMapEntry*
ConfigurationPolicyPrefKeeper::FindPolicyInMap(...) {
// <method body>
}

ConfigurationPolicyProvider*
ConfigurationPolicyProviderKeeper::device_management_provider() const {
return device_management_provider_.get();
}

Personally, I tend to prefer indentation, since it indicates that it's
a wrapping of what should ideally be only one line; but you might also
argue that having no indentation is more readable (especially if
you've seen that style convention elsewhere before).

Is there any agreement, or at least a majority vote, on this?

Jakob

[Jonathan Dixon]

My experience has been that different areas have a local convention. IIRC I've had review comments to use style #2 in chrome/browser/views, but style #1 in net/  both indicating it as the local convention rather than just that reviewer's personal choice

It would be nice to have a consistent style across all the project to aid developer portability, but it's not a major pain point. FWIW from previous projects you can add me to the group more familiar with the no-indent style. Not sure if this reflects a wider google style preference or not.

A related question: if it's a template class, where to prefer to break the line? (and indent, or not)?

[John Abd-El-Malek]

I've seen plenty of examples of 1 in the code.  I don't care as much either way, but I agree we really need to be consistent.

--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev

[Mike Lawther]

fwiw - the Google C++ Style Guide
(http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Line_Length)
only says "The return type is always on the same line as the function
name."

If style 1 is most prevalent, then I reckon stick with that one.

[Peter Kasting]

On Thu, Dec 23, 2010 at 3:03 AM, Jakob Kummerow <jkum...@chromium.org> wrote:

Note 1: These aren't my code, so please don't tell me to just rename
something to make it shorter.
Note 2: Observe that I formatted the second example with indentation,
the first without, to demonstrate the difference.

const ConfigurationPolicyPrefKeeper::PolicyToPreferenceMapEntry*
ConfigurationPolicyPrefKeeper::FindPolicyInMap(...) {
 // <method body>
}

ConfigurationPolicyProvider*
   ConfigurationPolicyProviderKeeper::device_management_provider() const {
 return device_management_provider_.get();
}

Use style 2.  It is more consistent with how we deal with all other wrapping issues.  IMO it is also slightly more readable.

I enforce style 2 in all my code reviews and I change any instances of 1 to 2 in the codebase when I come across them.

PK 

[Anton Vayvod]

On Fri, Dec 24, 2010 at 10:36 PM, Peter Kasting <pkas...@chromium.org> wrote:

On Thu, Dec 23, 2010 at 3:03 AM, Jakob Kummerow <jkum...@chromium.org> wrote:

Note 1: These aren't my code, so please don't tell me to just rename
something to make it shorter.

Well, you can tell that to people who own the code, right?

 

Note 2: Observe that I formatted the second example with indentation,
the first without, to demonstrate the difference.

const ConfigurationPolicyPrefKeeper::PolicyToPreferenceMapEntry*

Typedef in cc file could be used for the return type to shorten the line.

ConfigurationPolicyPrefKeeper::FindPolicyInMap(...) {
 // <method body>
}

ConfigurationPolicyProvider*
   ConfigurationPolicyProviderKeeper::device_management_provider() const {

I guess there should be 4 spaces indent, not 2, right? 

 return device_management_provider_.get();
}

Use style 2.  It is more consistent with how we deal with all other wrapping issues.  IMO it is also slightly more readable.

I enforce style 2 in all my code reviews and I change any instances of 1 to 2 in the codebase when I come across them.

PK 

--

[Peter Kasting]

On Mon, Dec 27, 2010 at 4:04 AM, Anton Vayvod <ava...@chromium.org> wrote:

Typedef in cc file could be used for the return type to shorten the line.

We could definitely use typedefs more often in general.  Not sure why people don't.

I guess there should be 4 spaces indent, not 2, right?

Yes.

PK