Open brace placement: always on the same line?

[Almale]

The Horstmann brace style should also be considered | discussed.

[jakevn]

Always thought it was a shame that Horstmann-style never spread.

It will likely remain obscure since it’s both different and unsupported by formatting tools.

[eridius]

The recommended indentation rules for line continuations already make it easy to visually scan line continuations and tell where the sequence of continuations stops in most cases. If a line continuation is is long and complicated enough that it’s hard to read, it should probably be broken up into multiple statements instead. Given that, I think a rule like the one @tbu quoted would actually decrease readability in most cases.

FWIW, @bachm’s supporting example is actually an example of a mis-indented line continuation. Corrected, it looks like

fn very_long_function(&mut self, long_name: &LongType, longer_name: LongerType, 
                      longest_name: LongerType) -> Vec<LongType> {
    codeGoesHere();
    // ...
}

and this is plenty easy to ready.

[tbu]

I don’t see how

fn very_long_function(&mut self, long_name: &LongType, longer_name: LongerType, 
                      longest_name: LongerType) -> Vec<LongType>
{
    codeGoesHere();
    // ...
}

would decrease code readability.

[eridius]

Readability is obviously a subjective thing. But if the coding standard says the brace should go on the same line and not a new one, putting it on a new one is an unexpected change that will negatively effect readability of anyone used to the recommended style. Every time you violate expectations you cause people to have to slow down and read more carefully to make sure they’re properly understanding the code. That’s basically the whole point of having guidelines in the first place.

Which is to say, if you’re going to have an exception to the standard rules, it has to have a really good justification to outweigh the exceptional nature.

[tbu]

Since we’re trying to develop the rules here, we’re basically building the expectation the reader will have.

[bachm]

Given that, I think a rule like the one @tbu quoted would actually decrease readability in most cases.

Let’s compare the currently recommended style with the style being proposed. For single line statements, they’re both identical. I’m adding some code to the function body to make it more realistic.

Multi-line statement, current style:

fn very_long_function(&mut self, long_name: &LongType, longer_name: LongerType, 
                      longest_name: LongerType) -> Vec<LongType> {
    self.method_call(long_name);
    for element in longest_name.iter() {
          self.do_stuff(element);
    }
    longer_name.make_vec(self)
}

Multi-line statement, proposed style without indenting to the level of the first argument (because the brace on the next line acts as delimiter):

fn very_long_function(&mut self, long_name: &LongType, longer_name: LongerType, 
    longest_name: LongerType) -> Vec<LongType> 
{ 
    self.method_call(long_name);
    for element in longest_name.iter() {
          self.do_stuff(element);
    }
    longer_name.make_vec(self)
}

Complicated or long type parameter lists also need to be considered. The current guidelines recommend the following:

fn very_long_function<T: Color + Weight, 
                      U: Direction>(
                      &mut self, 
                      long_name: &T, 
                      longer_name: LongerType, 
                      longest_name: U) 
                      -> Vec<T> {
    self.method_call(long_name);
    for element in longest_name.iter() {
          self.do_stuff(element);
    }
    longer_name.make_vec(self)
}

Proposed style:

fn very_long_function<T: Color + Weight, U: Direction>(&mut self, long_name: &T, 
    longer_name: LongerType, longest_name: U) -> Vec<T>
{
    self.method_call(long_name);
    for element in longest_name.iter() {
          self.do_stuff(element);
    }
    longer_name.make_vec(self)
}

The where clauses RFC is also relevant to this discussion. With where clauses, the code from above could be written as

fn very_long_function<T,U>(&mut self, 
                      long_name: &T, 
                      longer_name: LongerType, 
                      longest_name: U) 
                      -> Vec<T> 
                      where T: Color + Weight,
                            U: Direction {
    self.method_call(long_name);
    for element in longest_name.iter() {
          self.do_stuff(element);
    }
    longer_name.make_vec(self)
}

I’m guessing here how where clauses would be indented. In the RFC, they are used like this:

fn very_long_function<T,U>(&mut self, 
                           long_name: &T, 
                           longer_name: LongerType, 
                           longest_name: U) 
                           -> Vec<T> 
    where T: Color + Weight,
          U: Direction
{
     ...
}

In the proposed style, they could be used like this

fn very_long_function<T,U>(&mut self, long_name: &T, longer_name: LongerType,
    longest_name: U) -> Vec<T> 
    where T: Color + Weight,
          U: Direction
{
     ...
}

Obviously I prefer the proposed style, or at least my take on it.

PS: For some real examples, the Deque trait is written in the recommended style (and quite frankly, I find it dificult to read). Finally, looking at the std, indenting subsequent arguments to the level of the first argument is rare - evidently most users prefer a more traditional way to indent.

[arcto]

For ease of scanning and discerning parameters from function body I argue that these two sections should have different levels of indentation:

fn very_long_function<T, U>(
        &mut self, 
        long_name: &T, 
        longer_name: LongerType, 
        longest_name: U)
    -> Vec<T> 
    where
        T: Color + Weight,
        U: Direction
{
    self.method_call(long_name);
    for element in longest_name.iter() {
        self.do_stuff(element);
    }
    longer_name.make_vec(self)
}

Here, placing the opening brace on its own line helps distinguish the sections.

A more compact style is the Horstmann brace style (linked to by @Almale), which I find surprisingly refreshing:

fn very_long_function<T, U>
    (   &mut self, 
        long_name: &T, 
        longer_name: LongerType, 
        longest_name: U,
    ) -> Vec<T> 
    where
        T: Color + Weight,
        U: Direction
{   self.method_call(long_name);
    for element in longest_name.iter()
    {   self.do_stuff(element);
    }
    longer_name.make_vec(self)
}

[eridius]

Your first code block looks much better to me than your second. Not just because of the bracket placement, but also because your second violates the indentation rules. That violation makes it significantly harder to scan regardless of brace placement.

In your second code block, you have put in far too many line breaks. There is no need to put every parameter on its own line. There are a few options for how to actually wrap this, but I would probably do the following (given the current 99 char limit):

fn very_long_function<T: Color + Weight, U: Direction>(&mut self, long_name: &T,
                                                       longer_name: LongerType, longest_name: U)
                                                      -> Vec<T> {
    self.method_call(long_name);
    for element in longest_name.iter() {
        self.do_stuff(element);
    }
    longer_name.make_vec(self)
}

We don’t have any guidelines for indenting a where clause, but I would find it reasonable to perhaps indent it two or three levels, as in

fn very_long_function<T,U>(&mut self, long_name: &T, longer_name: LongerType, longest_name: U)
                          -> Vec<T> 
            where T: Color + Weight, U: Direction {
    ...
}

For some real examples, the Deque trait is written in the recommended style

Not sure what you mean. The trait is actually defined here, and no methods in it are long enough to wrap. What you linked to is just a few benchmark-related functions (which, incidentally, are also unnecessarily wrapping early, they could all fit on one line just fine with the 99 character limit).

[Almale]

Guys, you are starting to mix the brace-styles: use different ones for the function bodies and other blocks. I worry about this. Do you not think whichever style is chosen should be applied everywhere?

[tbu]

The proposal I made is a consistent rule for all cases.

[eridius]

No, the proposal you made covers all cases. But it’s explicitly not consistent. “Consistent” means all cases are handled the same. Your proposal splits it into two different rules that each apply to a non-overlapping subset of cases.

Consistency is king when it comes to style rules. It enforces simplicity, so you never have to think about when to apply one style and when to apply another, and it ensures everyone’s expectations for reading and writing code are the same.

[bachm]

If we go by that definition of consistent, the proposed rules are not any more inconsistent then the current ones. The inconsistency of the current rules is just in the indentation of arguments, not opening brace.

fn short(a: Bar, b: Bar) {

}

fn very_long_function(long_name: VeryLongType,
                      longer_name: VeryLongType,
                      longest_name: VeryLongType)
                      -> Bar {
    ...
}

So there’s little point arguing about consistency. Most will probably agree that one-line statements should be formatted differently than multi-line statements.

[eridius]

How is that inconsistent at all? That’s 3 rules right here:

1. Brace goes at the end
2. Line limit of (currently) 99
3. Line continuations are indented according to the line continuation indent rules

None of them contradict each other. There’s no case where one rule applies some of the time but not at other times (the closest argument to that is that line continuation rules don’t apply when there aren’t line continuations, but that’s like saying the brace-at-the-end rule doesn’t apply when there’s no brace).

[bluss]

I prefer to always have the brace on the next line, 1st column for impl scopes, functions and methods.

impl<T> Clone for Foo<T>
{
    fn clone(&self) -> Foo<T>
    {
         if self.frob {
           ...
         }
    }
}

Brace on the same line makes sense when the “header” and the block together form an expression, as they do with if clauses, but making a different recommendation for multiline “headers” sounds good to me.

[tbu]

These rules do indeed not contradict each other and may well be consistent.

I think I see a problem with that consistency here – namely, with a line limit like 99 character and line continuations rules that have >4 indent, we can pretty quickly get to the right border.

I’ve hit that case a few times with my code, and this is why I read this proposal and thought it was pretty good, as it does IMO not sacrify readability, keeps shortness for the common case and IMO even makes the many parameters block more readable.

[tbu]

Consistency is king when it comes to style rules. It enforces simplicity, so you never have to think about when to apply one style and when to apply another, and it ensures everyone’s expectations for reading and writing code are the same.

I think I disagree here. I wouldn’t sacrifice what is readability to me, for consistency.

(The PEP8 quote about consistency comes to my mind when reading that we should pursue consistency for the sake of it.)

[eridius]

I think I see a problem with that consistency here – namely, with a line limit like 99 character and line continuations rules that have >4 indent, we can pretty quickly get to the right border.

The move towards RAII in Rust was primarily motivated by a desire to get rid of rightward drift. This is also one of the primary motivating factors of my if-let RFC. No matter what the style rules are, rightward drift is generally considered bad and should be avoided when possible.

Also, I don’t see what rightward drift has to do with the placement of the open brace.

[bluss]

I think it was also that closures didn’t compile as fast or well as hoped, and made coding harder by involving borrowing into closures for scoping a lot more.

[aturon]

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.