ReSharper and StyleCop vs. the step-down rule (Clean Code)

Question

I imagine that this may be a bit of a divisive post, but it's something I've been struggling to articulate for a while, and I'd like to put it to the wider development community.

I work in a role where, before committing a check in on a file, we run the ReSharper auto format tool that groups things in regions by access modifier and sorts the members inside this alphabetically. It basically follows the class layout pattern described in this post: Alphabetizing methods in Visual Studio, which people seem to be very keen on.

I'm happy to work with any coding standards, but I'm struggling to reconcile this approach with writing clean code, primarily because I'm strict about adhering to the step-down rule (Robert C Martin - Clean Code), and the alphabetizing breaks that.

The Step-Down Rule is described as follows:

We want the code to read like a top-down narrative. We want every function to be followed by those at the next level of abstraction so that we can read the program, descending one level of abstraction at a time as we read down the list of functions. I call this the step-down rule. The ideal number of arguments for a function is zero. Next comes one. Then two. Three arguments should be avoided where possible.

Following this approach, I might write the following (contrived) code:

public class Processor
{
    public Processor(ProcessData data)
    {
        Configure(data);
    }


    public void Configure(ProcessData data)
    {
        ClearState();
        UnpackData();
        ProcessData();
        TransformData();
        PostProcessData();
    }

    private void ClearState() { /*Snip*/ }

    private void UnpackData() { /*Snip*/ }

    private void ProcessData() { /*Snip*/ }

    private void TransformData() { /*Snip*/ }

    private void PostProcessData() { /*Snip*/ }


    public IEnumerable<GroupedRecordSet> AggregateRecords(IEnumerable<Record> records)
    {
        var groups = CalculateGrouping(records);
        PopulateGroups(groups, records);
        return groups;
    }

    private IEnumerable<GroupedRecordSet> CalculateGrouping(IEnumerable<Record> records) { /*snip*/ }

    private void PopulateGroups(IEnumerable<GroupedRecordSet> groups, IEnumerable<Record> records) { /*snip*/ }
}

Then, when I run auto format, I end up looking at the following (Comments removed):

public class Processor
{
    #region Constructors and Destructors

    public Processor(ProcessData data)
    {
        Configure(data);
    }

    #endregion

    #region Public Methods and Operators

    public IEnumerable<GroupedRecordSet> AggregateRecords(IEnumerable<Record> records)
    {
        var groups = this.CalculateGrouping(records);
        this.PopulateGroups(groups, records);
        return groups;
    }

    public void Configure(ProcessData data)
    {
        this.ClearState();
        this.UnpackData();
        this.ProcessData();
        this.TransformData();
        this.PostProcessData();
    }

    #endregion

    #region Methods

    private IEnumerable<GroupedRecordSet> CalculateGrouping(IEnumerable<Record> records) { /*snip*/ }

    private void ClearState() { /*snip*/ }

    private void PopulateGroups(IEnumerable<GroupedRecordSet> groups, IEnumerable<Record> records) { /*snip*/ }

    private void PostProcessData() { /*snip*/ }

    private void ProcessData() { /*snip*/ }

    private void TransformData() { /*snip*/ }

    private void UnpackData() { /*snip*/ }

    #endregion
}

Now, I find the second style much harder to understand at a glance, and I find myself going to unusual lengths to retain the readability of the first style, within the confines of the second. These include:

• Owner method name prefix - i.e. ConfigureClearState, ConfigureUnpackData, AggregateRecordsCalculateGroupings, AggregateRecordsPopulateGroups, etc. Which leads to long member names, particularly if the 'owned' methods require additional 'owned' methods of their own.
• De-factoring - moving code from small methods I originally refactored out, back into the method where it came from. Which leads to long methods.
• Partial classes - I haven't actually gotten to this point yet, but it is entirely possible that I will end up putting related methods into partial classes to keep them separate from the main body of code. Which fills the solution explorer with piles of code files.

Obviously, I'm not happy with any of these approaches, but as far as I can see they are the only real options to maintain legibility within the operating parameters.

Apparently, the second approach is the Microsoft house style, so I suppose my question(s) would be:

• Is it correct that the second approach is the Microsoft house style?
• If so - how does Microsoft maintain clean readable code in the second style?
• Has anyone else encountered this disparity, and what approaches have people used to achieve high readability?
• What are the general style preferences for writing clean code?

I found a copy of the Microsoft coding style: http://blogs.msdn.com/b/brada/archive/2005/01/26/361363.aspx

Answer 1

The Robert Martin approach targets readability. To fully enjoy the benefits you have to apply additional conventions or rules (e.g., naming, abandon comments for the sake of choosing meaningful and descriptive names, single responsibility, short methods, ...). Then you can read your code like a common text document. If indenting the next abstraction level function block enhances readability as well. This way you can express abstraction levels by your code format:

public void Level1Member()
{
    RunLevel2Member();
    RunAnotherLevel2Member();
}

    private void RunLevel2Member()
    {
        RunLevel3Member();
    }

        private void RunLevel3Member()
        {
            //....
        }

    private void RunAnotherLevel2Member()
    {
        //..
    }

Maybe you'll find yourself abusing your mousewheel when using the alphabetical style just to scroll up and down to get your context. On the other hand you don't need to maintain any indentions and abstraction levels when refactoring your code.

This are two different approaches having different targets. One likes to enhance readability (for humans) and lets you find methods by the program flow while the other likes you to find methods fast by their name. Alphabetical sorting supports the common convention to place all public methods together which also increases the chance to figure out the purpose and behaviour of a class (at a glance). Step-down happily mixes public and private methods following a different aim.

You can't have both. So under no circumstances violate hundreds of rules and omit refactoring just to mix up this two styles. Makes no sense and makes the code much less readable.

If you think reading the step-down style feels more comfortable and natural than reading code in a dictionary style you should use it. I do.

Never heard of Microsoft internal conventions like sorting their code alphabetically. The official .NET convention doesn't target an organisation or convention for the structure of code (beside e.g. recommending to place event delegate at the top of a class, etc.).

Step-down rule is just an unofficial style. Some auto-formatting tools don't support indenting methods and places them back on one level.

By the way, using partial classes to obscure bad design (too big classes/ too much responsibility) is not an option for somebody caring about clean and maintainable code.

To make things easier for yourself I would try to show your team the advantage of your style or accept the style the majority of your team prefers.

Don't forget that your IDE assists you very well by highlighting the methods in your code or providing features like 'go to implementation' or 'find usages' or code maps showing the order of method calls. And there are rules of much more importance regarding code readability like good names and good design. But I personally prefer step-down.