C# Traps and Traps

Recently, on behalf of my employer, I spoke at the IT Academics Day, a small .NET conference organized by the West Pomeranian University of Technology. Together with Mateusz, we have presented some of the traps that a C# developer might encounter and how to recover from those. I would like to share three of ten such traps in today’s blog post, the rest will follow.

When inheritance is dangerous

The first one is a dangerous design trap, a bomb with a long fuse. Let’s take a look at the following code:

internal class BaseData
{
   protected string data = null;

   public BaseData(string data)
   {
       this.data = data;
       this.InitializeState();

       Console.WriteLine("BaseData created");
   }

   public virtual void InitializeState()
   {
       // Some basic initialization logic.
   }
}

Do you see anything dangerous yet?

Anyhow, since we have named the class BaseData there is nothing else to do but to extend it. First, the constructor. We have to call the base class constructor and pass our data parameter. Then we store the source parameter and we are good to go. Last but not least, we would like to override the InitializeState method from the BaseData and check the state of one of the parameters.

internal class NetworkData : BaseData
{
   private string source = null;

   public NetworkData(string data, string source) : base(data)
   {
      this.source = source;
      Console.WriteLine("NetworkData created");
   }

   public override void InitializeState()
   {
      if (source.Contains("someString"))    
      {

      }
  }
}

Right, so you probably see the problem… first of all, we need to search through our memory and remember what will be executed while creating an instance of NetworkData. First, we call the NetworkData constructor – inside of it we call the base class constructor, then the base class constructor is executed and afterward we execute NetworkData constructor. But because we have overridden virtual method InitializeState, our BaseClass constructor will call InitializeState from NetworkData class, because we called it from NetworkData context. And because of that, the BaseClass constructor will throw null reference exception because we have yet to assign the source member variable. We have tried to, but actually the NetworkData constructor is too late in the execution order for us to initialize the object properly.

So the lesson here is pretty straightforward. We should avoid calling virtual methods in the constructor. The simplest rationale for this is that this method will be executed in the context of the uninitialized object. And we set ourselves for failure, our code will crash because of someone else’s code.

Lambda “gotcha” expression

The second one is a really classic example of the mismatch between how you read the code and how it is executed. I have discovered it the hard way – during long and frustrating hours of debugging. Consider the following code:

var actionsFor = new List<Action>();
for (int number = 0; number < 3; ++number)
{
    actionsFor.Add(() => Console.WriteLine(number));  
}
            
for (int index = 0; index < 3; ++index)
{
    actionsFor[index]();
}

Nothing fancy, we create a list of delegates and we populate it with lambda expression inside the for loop, with intent to print current iteration index. After that, we iterate through the collection and execute the stored action. Great, we managed to print 0 1 2 in a really sophisticated way!

After testing the code we notice that the console output showed actually 3 3 3 which is not what we want. Wait, what? I remember the confusion after seeing this. Let’s not start to panic, luckily the code is really simple and easy to understand. The first clue is the printed value itself, it must have come as a result of incrementation – in fact, 3 is a value that is a result of incrementation after the last loop execution. So the error is in fact in the first loop, where we store action that captures number variable. Lambda is translated into a class that stores (captures) variable and action – and the variable will be stored by reference, not by value. In another word, when executing stored action we will use a reference to number, which at the time of execution (in the second loop) will have a value of 3.

To fix this code we need to introduce a temporary variable that could be captured inside the lambda expression. The following code works as intended:

for (int number = 0; number < 3; ++number)
{
    var copyOfNumber = number;
    actionsFor.Add(() => Console.WriteLine(copyOfNumber));
}

Let’s dive a bit deeper and check a behavior of the foreach loop. If we rewrite the original example to use the foreach loop, will it work?

var actionsForeach = new List<Action>();

foreach (var i in Enumerable.Range(0, 3))
{
    actionsForeach.Add(() => Console.WriteLine(i));
}
            
foreach (var action in actionsForeach)
{
    action();
}

Life of a software engineer is never easy as there isn’t a yes-no answer here. It does depend on the C# version you are using. With the introduction of C# 5 (it came with Visual Studio 2012), Microsoft had made a breaking change and the code work as we expect. It is because the i variable is created inside the scope of the foreach loop, a new instance is made every iteration, so the lambda does capture different reference each time. Ok then, so why was that not fixed for the for loop? Microsoft explained the reasoning in a very open and convincing way, it is a great read – https://blogs.msdn.microsoft.com/ericlippert/2009/11/12/closing-over-the-loop-variable-considered-harmful/.

Just build it again…

The third one is a problem of a different kind – it is this beast of a bug that appears seemingly at random, then somehow it is fixed, but then it is back. First of all, the setup. We execute the following code in an assembly (let’s call it A), but the SensorData class is in different assembly (B), and the ConsolePresenter is in yet another assembly (C). We will execute the following code, with no changes:

var speed = new SensorData("Depth", 15.1234, "meters");     
var presenter = new ConsolePresenter();
            
presenter.Show(speed);

The ConsolePresenter is really simple, we have one method that takes SensorData and prints it to the console using the ToFormattedString method, please note no parameters in the ToFormattedString method.

public class ConsolePresenter
{
    public void Show(SensorData data)
    {
        Console.WriteLine(data.ToFormattedString());
    }
}

Lastly, the SensorData class:

public class SensorData
{
    public string Name { get; }
    public double Value { get; }
    public string Unit { get; }
    public SensorData(string name, double value, string unit)
    {
        this.Name = name;
        this.Value = value;
        this.Unit = unit;
    }

    public string ToFormattedString(string format="{0}: {1:0.00} {2}")
    {
        return string.Format(format, this.Name, this.Value, this.Unit);
    }
}

Please note that the ToFormattedString is using default parameters for the only parameters – inside the ConsolePresenter we use those default values to print the SensorData.
When the code is executed, we will get Depth: 15.12 meters printed to the console. As expected – great! So let’s modify the behavior to display only one digit after decimal point. To do so, we will modify the ToFormattedString method inside the SensorData class, and change it’s default parameter to reflect our desired behavior. Great, now let’s compile the assembly (B) and run it. We get the old behavior – 15.12. At this point, we would not panic yet because intuitively we know we should recompile the application (A) and try again. Still, nothing has changed, as if our modification was not saved yet. By now we would be in the debugger, hitting breakpoints and verifying we are not crazy and we did modify the code. Yes, our debugging sessions are clearly showing us that we are using code that we have modified, but yet it does not work. We hit the “rebuild all” and hope for the best – and this time, it does work! Great, or is it? What is it magical about a “rebuild all”?

Well, there is nothing magical, we just didn’t rebuild the project that holds ConsolePresenter yet, and rebuild did all that for us. Normally you would think that there is no need to rebuild the DLL since we didn’t modify it and you would be wrong – we did modify it. It turns out that the default parameters of a method are considered it’s interface and they are compiled into every library that consumes the method.

So lesson to be learned here is – don’t use default parameters. Ok, I might be overreacting. We have only shown they are dangerous in public context, nothing wrong will happen if you keep them internal to the assembly. But you could also, you know, not use them…



        

Leave a Reply

Your email address will not be published. Required fields are marked *