dotnet-foundation / project-maturity-model

Proposal/RFC for new .NET library development model.
MIT License
39 stars 8 forks source link

NET Coding guidelines requirement #45

Open isaacabraham opened 5 years ago

isaacabraham commented 5 years ago

I'm not especially fond of the idea of this requirement. Again in relation to what has been mentioned in other threads, I'm not sure it's really appropriate. I quote from the guidelines:

They started as a small set of naming and design conventions but have been enhanced, scrutinized, and refined to a point where they are generally considered the canonical way to design frameworks at Microsoft.

What might be appropriate for a framework designed by Microsoft may not (and indeed probably is not) appropriate for the entire NET community at large. Even if they were, making all codebases conform to a single style might not be the right thing to do in general. Again, this feels more like a ladder that makes it easier for projects to be adopted by Microsoft.

Also, it would need to be adjusted to apply to F# and possibly VB as well.

I get that by doing this it increases the chances of more people and projects getting involved in this programme, but this feels overly subjective to me.

glennawatson commented 5 years ago

Maybe having a coding standard as a requirement would be better rather than a specific one and having the .net one listed as an example. Maybe better yet having the foundation hosting the coding standard suggestion as the recommendations rather than the FX core one.

glennawatson commented 5 years ago

I think also moving to a central set of recommending coding standards rather than using FX core will help with the perception that the .net foundation is independent of Microsoft.

benaadams commented 5 years ago

I don't feel to strongly about any particular coding standards internal to a project; however I can see advantages for a consumer of a library if its public api follows expected conventions.

Saying that it would be more of a preference or guideline rather than a requirement; don't think we'd want to stifle innovation in this area.

haacked commented 5 years ago

Saying that it would be more of a preference or guideline rather than a requirement; don't think we'd want to stifle innovation in this area.

I agree with this. In part because whether you follow the coding standards or only has a little influence on whether I should depend on your library. If all other things are equal, I would prefer a library that followed the guidelines because I know what to expect, but I wouldn't require anything like this on any library.

isaacabraham commented 5 years ago

The more I'm thinking about this, the more I think that this could just become one huge bikeshedding exercise on a project-by-project basis. I've actually got a copy of the guidelines book here, but here are just a few examples taken from the online distillation which might have made perfect sense when the guidelines came out but nowadays could be considered controversial or muddied either through new features in C# or newer "best practices":

✓ DO prefer classes over interfaces.

The one is unclear anyway but further complicated through advances in C# such as default interface methods.

✗ DO NOT seal types unless you have a strong reason to do it.

I seem to recall Immo on Twitter this month talking about how classes should actually be sealed by default.

✓ DO prefer constructors over factory methods.

I read a conversation from several notable .NET community figures just this week discussing doing the exact opposite to this advice.

I also seem to recall a large section in the book on the use of the Template pattern as well which, especially with the rise in popularity of unit testing, is generally nowadays replaced with Strategy (or simple passing of higher order functions e.g. LINQ).

These are just a few examples based on a cursory glance through the list. Personally I would drop the enforcement of this completely, but if it's really necessary I would suggest that "less is more".

  1. Come up with some high-level, broad guidelines that are high level enough to provide flexibility and don't need to be updated and rewritten with every change to C# / VB / F#
  2. A few specific, low-level guidelines that are not controversial, very clear and very easy to enforce.
glennawatson commented 5 years ago

Seems the clauses you mentioned make sense when your dealing with a framework that needs to be optimised on a low level and is the core framework for a language.

I would prefer a project has a coding standard formalised personally over sticking to a specific one.

Also could imagine this could be a pain point of entry into the maturity profiles for new projects if we use strict corefx coding standards.

More I think about it the more this part of the proposal should be less strict on using that coding standard.

benaadams commented 5 years ago

I think adding more detail on the "why"s would add some clarity and also frame the debate a bit better; as @JamesNK brings up in https://github.com/dotnet-foundation/project-maturity-model/issues/30

isaacabraham commented 5 years ago

@glennawatson Could be - I'm pretty sure Immo was referring to sealing classes with relation to netcore / framework though.

In principle I can see the benefits, but in reality I'm struggling to see how this should realistically be enforced.