Complexity hunt

July 30, 2022 on Maarten Vos's Blog

Programming languages are getting too complex for their own good. Not only is it difficult to remember every new feature of your programming language, previously written code also starts to look unidiomatic. But that’s not the worst of it. When you start to rely on these new fancy features, you are hiding important details of your code. Or, in the following case, introduces a bug you have to actively hunt for.

public override Stream OnGetStream(GetStreamArgs e)
{
    var stream = new MemoryStream();
    
    using var zip = new ZipArchive(stream, ZipArchiveMode.Create, true);

    stream.Position = 0;
    
    return stream;
}

Take the previous code block for example, this is complete valid C# and there doesn’t appear to be anything wrong with it. It uses the new using syntax introduced in C# 8. However, if you were to run this code, you would notice that what you get back is an empty archive at best, or an invalid one at worst. Why is that you may ask? Well, the answers lays in how the ZipArchive class works. The ZipArchive class only writes its data to the underlying stream once it has been disposed. You may think, but that’s what the using statement does, and you would be right. However, the new using syntax comes with a caveat. It brings your variable into the scope of the method.

The using statement wraps your code into a try-finally block, where the object is diposed. However, this happens at the end of the scope, i.e the method. Before that happens, we reset the stream’s position so that it can be read, but disposing of our ZipArchive moves the Position ahead for more writing. If you remember how the ZipArchive class works, it only writes its data to the underlying stream once its been disposed. What you get is code that looks perfectly fine, but is broken because some fancy new syntax doesn’t behave how you would expect it would (disposing right after the object was last used). I spent my morning at work figuring this out and while it wasn’t particularly difficult, it took time that I could have spent on more important issues.

public override Stream OnGetStream(GetStreamArgs e)
{
    var stream = new MemoryStream();
    
    using (var zip = new ZipArchive(stream, ZipArchiveMode.Create, true))
    {
    }

    // Or you could do this.
    //zip.Dispose();

    stream.Position = 0;
    
    return stream;
}

As you can see in the code above, the issue is solved easily. You use the old syntax, which introduces a new scope and ensures that the object is disposed before setting the stream’s position to 0, or you dispose it manually. I prefer using the old syntax since it introduces that new scope, making the code easier to follow since it’s clear where the ZipArchive is disposed.

With that, my hunt for complexity comes to an end…… until history repeats itself. Happy hunting!

Have a comment on one of my posts? Start a discussion in my public inbox by sending an email to ~inferiormartin/public-inbox@lists.sr.ht [mailing list etiquette]