Coding convention: braces

Jan 7, 2011 at 3:50 AM
(don't want to start a flame war, seriously)

but I wonder, why do the nuget team members find the current convention like this (from the actual source):

namespace NuGet {
using System;
using System.Runtime.Versioning;

public interface IPackageAssemblyReference : IPackageFile {
FrameworkName TargetFramework {
get;
}

string Name {
get;
}
}
}

nicer than the C# default (just do Add Class ;)):

using System;
using System.Runtime.Versioning;

namespace NuGet
{
public interface IPackageAssemblyReference : IPackageFile
{
FrameworkName TargetFramework { get; }
string Name { get; }
}
}

at least to my eye, the latter is WAY more readable (and what I've seen used in every project I worked on so far, but I haven't done any javascript in years...)

/kzu

--
Daniel Cazzulino | Developer Lead | MS MVP | Clarius Consulting | +1 425.329.3471
Jan 7, 2011 at 4:20 AM

You should try using the right most button on the toolbar when you paste code to preserve the formatting, since the point here is to compare formats. :)

I'll just say that personally I find spreading automatic props over multiple lines ridiculous.  Is that really our guideline?  I think a lot of our code has it one a single line.

Jan 7, 2011 at 4:23 AM

Damn email posting from gmail! :(

The formatting is really a mess. I can't really figure out to keep half with single line for automatic properties, and half with these crazy newlines ;)

namespace NuGet {
    using System;
    using System.Runtime.Versioning;

    public interface IPackageAssemblyReference : IPackageFile {
        FrameworkName TargetFramework {
            get;
        }

        string Name {
            get;
        }
    }
}

// vs

using System;
using System.Runtime.Versioning;

namespace NuGet
{
	public interface IPackageAssemblyReference : IPackageFile
	{
		FrameworkName TargetFramework { get; }
		string Name { get; }
	}
}

Coordinator
Jan 7, 2011 at 4:24 AM

The original guideline is based on our internal ASP.NET guideline. We have to pick some guideline, so we chose one. As we point out in the coding guidelines page:

Let’s face it. No matter what coding guidelines we choose, we’re not going to make everyone happy. In fact, some people out there might be downright angry at the choices we make. But the fact of the matter is that there is no “one true bracing style”, despite attempts to name a bracing style as such. While we would like to embrace everyone’s individual style, working together on the same codebase would be utter chaos if we don’t enforce some consistency. When it comes to coding guidelines, consistency can be even more important than being “right”.

Having said that, I don’t really care about the auto properties. On the one hand, why have a special formatting rule for them and not for anything else? The current guidelines are consistent across the board. On the other hand, if nobody likes the guideline for auto props, then we can choose to change it.

Overall, I find debates over bracing styles to be unproductive and pointless. Let’s try to make this the first and the last one here.

Phil

Jan 7, 2011 at 4:28 AM

I'd say we officially dump the crazy auto prop formatting and move on.  Most of our code is not using it I think, so it's actually a smaller code change to change the guideline.  Case closed!  As if... ;)

Jan 7, 2011 at 4:28 AM
It's a matter of "commonality", I'd say. I have the feeling that the default C# bracing style is WAY more pervasive than this one.

I'm not arguing for a special case for automatic properties, but for everything.

A quick poll should suffice to determine what's more familiar for everyone (and not just the ASP.NET team ;)).

All I'm saying is that while your GUIDELINE may be consistent across the board, your CODE is far from consistent :P

/kzu

--
Daniel Cazzulino | Developer Lead | MS MVP | Clarius Consulting | +1 425.329.3471


On Fri, Jan 7, 2011 at 02:24, Haacked <notifications@codeplex.com> wrote:

From: Haacked

The original guideline is based on our internal ASP.NET guideline. We have to pick some guideline, so we chose one. As we point out in the coding guidelines page:

Let’s face it. No matter what coding guidelines we choose, we’re not going to make everyone happy. In fact, some people out there might be downright angry at the choices we make. But the fact of the matter is that there is no “one true bracing style”, despite attempts to name a bracing style as such. While we would like to embrace everyone’s individual style, working together on the same codebase would be utter chaos if we don’t enforce some consistency. When it comes to coding guidelines, consistency can be even more important than being “right”.

Having said that, I don’t really care about the auto properties. On the one hand, why have a special formatting rule for them and not for anything else? The current guidelines are consistent across the board. On the other hand, if nobody likes the guideline for auto props, then we can choose to change it.

Overall, I find debates over bracing styles to be unproductive and pointless. Let’s try to make this the first and the last one here.

Phil

Read the full discussion online.

To add a post to this discussion, reply to this email (nuget@discussions.codeplex.com)

To start a new discussion for this project, email nuget@discussions.codeplex.com

You are receiving this email because you subscribed to this discussion on CodePlex. You can unsubscribe or change your settings on codePlex.com.

Please note: Images and attachments will be removed from emails. Any posts to this discussion will also be available online at codeplex.com


Coordinator
Jan 7, 2011 at 4:35 AM

As I said, this is a pointless argument. We can change the auto props, as for everything else, we can debate it on meta.stackoverflow.com till the sun swallows the Earth.

Phil

Jan 7, 2011 at 4:45 AM

consistency is key. you do it for everything, or for nothing. (not only the guideline, but your actual code, ideally)

current codebase looks like:
using System;
using System.Net;

namespace NuGet {
    public interface IHttpClient {
        string UserAgent {
            get;
            set;
        }

        WebRequest CreateRequest(Uri uri);
        void InitializeRequest(WebRequest request);
        Uri GetRedirectedUri(Uri uri);
    }
}


------------------------------

namespace NuGet {
    using System;
    using System.Runtime.Versioning;

    public interface IPackageAssemblyReference : IPackageFile {
        FrameworkName TargetFramework {
            get;
        }

        string Name {
            get;
        }
    }
}
------------------------------
using System.IO; namespace NuGet { internal class PathSearchFilter { public PathSearchFilter(string searchDirectory, string searchPattern, SearchOption searchOption) { SearchDirectory = searchDirectory; SearchPattern = searchPattern; SearchOption = searchOption; } public string SearchDirectory { get; private set; } public SearchOption SearchOption { get; private set; } public string SearchPattern { get; private set; } } }
inconsistent. as a contributor, I'd much rather read code and see how it looks to know how I have to format mine, than read a guideline ;). even better would be a .settings file, haha
I guess I will have to spend a few more hours on the damn Quick Settings WoVS extension and attach the appropriate settings to the nuget solution and that's it :). everyone happy!
Jan 7, 2011 at 4:48 AM

[damn paste!]

 

using System.IO;

namespace NuGet {
    internal class PathSearchFilter {
        public PathSearchFilter(string searchDirectory, string searchPattern, SearchOption searchOption) {
            SearchDirectory = searchDirectory;
            SearchPattern = searchPattern;
            SearchOption = searchOption;
        }

        public string SearchDirectory { get; private set; }

        public SearchOption SearchOption { get; private set; }

        public string SearchPattern { get; private set; }
    }
}


So you have: usings inside and outside of the namespace declaration, auto props with braces on new lines and on same line, etc.

 

Coordinator
Jan 7, 2011 at 4:48 AM

Now that’s a productive idea! J

The world is never going to agree on a bracing style. I work on multiple projects each with its own. I’ve been bugging you about a WoVS extension to handle that for a while. I thought you had one already! :P

Phil

Jan 7, 2011 at 4:51 AM

yup, we have it, but as always, polishing the details takes longer than the core work (in this case, import/export dialog getting polluted with ALLLLL settings that are being auto-applied every time you open a solution :()

Anyway, if the goal is to help people to never go back to the Import/Export dialog, I might just ignore the issue :)

Coordinator
Jan 7, 2011 at 4:56 AM

Yeah, that’s a problem. We have some code that was written before we settled on the guidelines.


Phil

Jan 7, 2011 at 7:14 AM

Ok, I changed the auto prop guideline to be single line, which we use in most places anyway (though many don't, we're all over the place).

Jan 8, 2011 at 3:59 AM

K&R ftw.

Jan 8, 2011 at 4:04 AM

Allman ftw :P

Jan 8, 2011 at 4:50 AM

No, K&R. You suck.

;-)

Jan 8, 2011 at 12:10 PM

Haha :p

/kzu from galaxy tab

On Jan 8, 2011 2:50 AM, "oisin" <notifications@codeplex.com> wrote:
> From: oisin
>
> No, K&R. You suck.;-)
>
>