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

Circumvallate

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.

Nowt

I agree, and I also find the alphabetical sorting pretty redundant, as we already have numerous places in the IDE where we can access members alphabetically and jump straight to them. – Circumvallate

Also - not sure how I feel about indenting the code to make the steps physical, i have always tended to separate logical groups of method statements by a double white line instead. – Circumvallate

Yeah thats up to taste. I think when scrolling a page it is easier to determine the change of indention than the difference in line space (1 line) just to have clue where you're in code. More spaces between lines is known to be uncomfortable to read (eye movement). Indenting makes the abstraction levels more visible. The idea is that you just need to read the first level (non-indented) and maybe the second level methods to understand the basic function of your code. Every further indention or abstraction level reveals more of the details of how the basic function of that class is accomplished. – Nowt

The stepdown rule is impossible in languages that require or recommend thatba function be declared before it is used. Ie javascript. – Redman

@Ray, like in C/C++, you have declaration and definition. The compiler needs to to know about definitions BEFORE you can actually use them. Declaration doesn't have a body. It's in the definition of that declared function. In this case you just have to keep all declarations in one place (e.g. header file) and move definitions to the source file. You can sort and arrange to meet your preferences in both files. You named JavaScript. JavaScript only knows 'declaration' which is like a definition in C/C++. JavaScript (like Java, C#) allows you to put 'definitions' wherever you like in any order. – Nowt

@Ray, you refer to forward declaration i guess. – Nowt