Partitioning and aligning long code lines

Formatting the code is very important for readability. And it is for a reason that in magazines and newspapers we have few narrow columns on the page. It is much easier to read text when you don’t have to move your eyes back and forth. However, when we code in a statically typed language like C# it is not always easy to make code narrow. Types have to be declared for every variable and parameter, so we have to break statements into several lines. Doing this in a wrong way, however, one can make more harm than good. So, let’s start considering code without breaks at all:

public class MyClass {
    public void MyMethod(LongTypeName1 longParamName1, LongTypeName2 longParamName2, LongTypeName3 longParamName3) {
        //Some logic goes here
        //Some logic goes here
        //Some logic goes here
    }
}

Reading this can be challenging. You have to move your eyes on the right and then back, distracting yourself from an ordinary top-down flow. Sometimes you even have to grab a mouse and scroll. Lame…
Here is what people start doing with this:

public class MyClass {
    public void MyMethod(LongTypeName1 longParamName1,
        LongTypeName2 longParamName2, LongTypeName3 longParamName3) {
        //Some logic goes here
        //Some logic goes here
        //Some logic goes here
    }
}

And this a mistake! Because longParamName1 is now visually detached from other parameters while logically all parameters are related and should be visually grouped together. In fact, this is the fundamental principle: logically related entities should be related visually. Now if you agree with this statement you might fix the problem in the following way:

public class MyClass {
    public void MyMethod(LongTypeName1 longParamName1,
                         LongTypeName2 longParamName2,
                         LongTypeName3 longParamName3) {
        //Some logic goes here
        //Some logic goes here
        //Some logic goes here
    }
}

Now the fundamental principle is met and we can easily see the grouping. However, there is a hidden mistake. What would happen if you rename the method? Most probably longParamName1 will be shifted left or right while all other parameters will keep their previous alignment. This change will lead to the previous problem – visual detachment of logically related entities.
This can be fixed in the following way:

public class MyClass {
    public void MyMethod(
        LongTypeName1 longParamName1, LongTypeName2 longParamName2, LongTypeName3 longParamName3) {
        //Some logic goes here
        //Some logic goes here
        //Some logic goes here
    }
}

This code solves previous visual detachment problems, but if the parameters list is long, we still have the problem. Luckily enough there is formatting which solves most of described problems:

public class MyClass {
    public void MyMethod(
        LongTypeName1 longParamName1,
        LongTypeName2 longParamName2,
        LongTypeName3 longParamName3)
    {
        //Some logic goes here
        //Some logic goes here
        //Some logic goes here
    }
}

Note that method’s opening figure bracket is placed on the new line. This visually detaches logically unrelated entities – code of the function and list of its parameters. This same rule can be applied to other things in code, to LINQ expressions for instance. It is better to format LINQ expression this way:

new[] { 1, 2, 3, 4 }
    .Where(n => n > 3)
    .OrderBy(n => n)
    .Take(3)
    .Select(n => n * n);

than this way:

new[] { 1, 2, 3, 4 }.Where(n => n > 3)
    .OrderBy(n => n)
    .Take(3)
    .Select(n => n * n);

Please refer here if you would like to learn more on the topic.

Instead of conclusion: before analyzing different formattings and selecting better one, my developer’s life was a complete mess. I spent much time formatting and reformatting things and still being unsatisfied with the visual appearance of my C# code. Now I have no hesitations and even legacy code can be a bit beautified, which is always great.

Facebooktwittergoogle_pluslinkedinmailFacebooktwittergoogle_pluslinkedinmail

Boolean parameters considered harmful

A lot has been said on the harm of Boolean parameters. And it seems that this harm is absolute truth without any “it depends”. Recently I had a little accident at my work, which was very much related to Boolean parameter and my own laziness. In C#, I had a class, which I wanted to instantiate. I also wanted that class to be able to act in a little bit different manner, depending on the setting, passed to its constructor. It happened to be 2 different settings possible and I thought, that here was exceptional case and I would better go with Boolean parameter. I made this decision, acted, checked-in. No, I did not unit-test. It was just simple constructor, and simple API to invoke that constructor. It was all great until this Boolean hunted me down few months later.¬† This is what happened. I was instantiating MyClass using Activator.CreateInstance, whose signature was the following:

public static object CreateInstance(Type type, params object[] args)

This is it. Very simple, I just ask to create me an instance of MyClass using its constructor with Boolean parameter. What do you think is the output of the following code?

public static class Program {
    public static void Main() {
        Console.WriteLine(MyClassCreator.CreateWithTrue().MyProperty);
    }
}

public static class MyClassCreator {
    public static MyClass CreateWithTrue() {
        return (MyClass)Activator.CreateInstance(
            typeof(MyClass), 
            true);
    }
    public static MyClass CreateWithFalse() {
        return (MyClass)Activator.CreateInstance(
            typeof(MyClass), 
            false);
    }
}

public class MyClass {
    public bool MyProperty { get; set; }
    public MyClass() { }
    public MyClass(bool myProperty) {
        MyProperty = myProperty;
    }
}

Although I wanted my instance to have MyProperty initialized to “true”, it was actually initialized to “false”. And it was so because of¬†another a bit lazy guy in Microsoft, or whatever company which wrote Activator, who decided to overload CreateInstance in the following way:

public static object CreateInstance(Type type, bool nonPublic)

Now, when compiler performs linking the call to CreateInstance, it choses the overload with closest match and it is the one with Boolean parameter. Hence the wrong method got called and in reality default constructor was used to instantiate MyClass.

Now, of course, you can object and say that one has to look carefully to what he/she invokes. While this is true, it is often really easy to miss something, and therefore it is much better to protect yourself as much as you can. If only I used enumeration with two possible values, this clash would not have happened:

public enum Options { OptionTrue, OptionFalse }

public static class MyClassCreator {
    public static MyClass CreateWithTrue() {
        return (MyClass)Activator.CreateInstance(
            typeof(MyClass),
            Options.OptionTrue);
    }
    public static MyClass CreateWithFalse() {
        return (MyClass)Activator.CreateInstance(
            typeof(MyClass),
            Options.OptionTrue);
    }
}

By the way, this error was not detected on the test, and in fact introduced security issue. And only diligent developer, who wanted to reuse MyClassCreator, stumbled across this strange code with Boolean literals and raised an alarm.

Conclusion: if you want to use a Boolean parameter to whatever method, think twice and be prepared to be hunted down later. Code with Boolean parameters not only makes you think hard to understand what does “true” or “false” might mean in the given context. It also can make you commit very nasty errors.

Facebooktwittergoogle_pluslinkedinmailFacebooktwittergoogle_pluslinkedinmail