All posts by jonskeet

Mad props to @arcaderage for the "Princess Rescue" image - see https://toggl.com/programming-princess for the full original
General

The Open-Closed Principle, in review

45 Comments

Background

I’ve been to a few talks on SOLID before. Most of the principles seem pretty reasonable to me – but I’ve never "got" the open-closed principle (OCP from here on). At CodeMash this year, I mentioned this to the wonderful Cori Drew, who said that she’d been at a user group talk where she felt it was explained well. She mailed me a link to the user group video, which I finally managed to get round to watching last week. (The OCP part is at around 1 hour 20.)

Unfortunately I still wasn’t satisfied, so I thought I’d try to hit up the relevant literature. Obviously there are umpteen guides to OCP, but I decided to start with Wikipedia, and go from there. I mentioned my continuing disappointment on Twitter, and the conversation got lively. Uncle Bob Martin (one of the two "canonical sources" for OCP) wrote a follow-up blog post, and I decided it would be worth writing one of my own, too, which you’re now reading.

I should say up-front that in some senses this blog post isn’t so much about the details of the open-closed principle, as about the importance of careful choice of terminology at all levels. As we’ll see later, when it comes to the "true" meaning of OCP, I’m pretty much with Uncle Bob: it’s motherhood and apple pie. But I believe that meaning is much more clearly stated in various other principles, and that OCP as the expression of an idea is doing more harm than good.

Reading material

So what is it? (Part 1 – high level)

This is where it gets interesting. You see, there appear to be several different interpretation of the principle – some only subtly distinct, others seemingly almost unrelated. Even without looking anything up, I knew an expanded version of the name:

Modules should be open for extension and closed for modification.

The version quoted in Wikipedia and in Uncle Bob’s paper actually uses "Software entities (classes, modules, functions, etc.)" instead of modules, but I’m not sure that really helps. Now I’m not naïve enough to expect everything in a principle to be clear just from the title, but I do expect some light to be shed. In this case, unfortunately I’m none the wiser. "Open" and "closed" sound permissive and restrictive respectively, but without very concrete ideas about what "extension" and "modification" mean, it’s hard to tell much more.

Fair enough – so we read on to the next level. Unfortunately I don’t have Bertrand Meyer’s "Object-Oriented Software Construction" book (which I take to be the original), but Uncle Bob’s paper is freely available. Wikipedia’s summary of Meyer’s version is:

The idea was that once completed, the implementation of a class could only be modified to correct errors; new or changed features would require that a different class be created. That class could reuse coding from the original class through inheritance. The derived subclass might or might not have the same interface as the original class.

Meyer’s definition advocates implementation inheritance. Implementation can be reused through inheritance but interface specifications need not be. The existing implementation is closed to modifications, and new implementations need not implement the existing interface.

And Uncle Bob’s high level description is:

Modules that conform to the open-closed principle have two primary attributes.

  1. They are "Open For Extension". This means that the behavior of the module can be extended. That we can make the module behave in new and different ways as the requirements of the application change, or to meet the needs of new applications.
  2. They are "Closed for Modification". The source code of such a module is inviolate. No one is allowed to make source code changes to it.

I immediately took a dislike to both of these descriptions. Both of them specifically say that the source code can’t be changed, and the description of Meyer’s approach to "make a change by extending a class" feels like a ghastly abuse of inheritance to me… and goes firmly against my (continued) belief in Josh Bloch’s advice of "design for inheritance or prohibit it" – where in the majority of cases, designing a class for inheritance involves an awful lot of work for little gain. Designing an interface (or pure abstract class) still involves work, but with fewer restrictions and risks.

Craig Larman’s article uses the term "closed" in a much more reasonable way, to my mind:

Also, the phrase "closed with respect to X" means that clients are not affected if X changes.

When I say "more reasonable way" I mean in terms of how I want to write code… not in terms of the use of the word "closed". This is simply not how the word "closed" is used elsewhere in my experience. In the rare cases where "closed" is used with "to", it’s usually in terms of what’s not allowed in: "This bar is closed to under 18s" for example. Indeed, that’s how I read "closed to modification" and that appears to be backed up by the two quotes which say that once a class is complete, the source code cannot be changed.

Likewise the meaning of "open for extension" seems unusual to me. I’d argue that the intuitive meaning is "can be extended" – where the use of the term "extended" certainly nods towards inheritance, even if it’s not intended meaning. If the idea is "we can make the module behave differently" – as Uncle Bob’s description suggests – then "open for extension" is a very odd way of expressing that idea. I’d even argue that in the example given later, it’s not the "open" module that behaves differently – it’s the combination of the module and its collaborators, acting as a unified program, which behaves differently after some aspects are modified.

So what is it? (Part 2 – more detail)

Reading on through the rest of Uncle Bob’s paper, the ideas become much more familiar. There’s a reasonable example of a function which is asked to draw a collection of shapes: the bad code is aware of all the types of shape available, and handles each one separately. The good code uses an abstraction where each shape (Circle, Square) knows how to draw itself and inherits from a common base class (Shape). Great stuff… but what’s that got to do with what was described above? How are the concepts of "open" and "closed" clarified?

The answer is that they’re not. The word "open" doesn’t occur anywhere in the rest of the text, other than as part of the term "open-closed principle" or as a label for "open client". While it’s perhaps rather easier to see this in hindsight, I suspect that any time a section which is meant to clarify a concept doesn’t use some of the key words used to describe the concept in a nutshell, that description should be treated as suspect.

The word "closed" appears more often, but only in terms of "closed against" which is never actually defined. (Is "closed against" the same as "closed for"?) Without Craig Larman’s explanation, sentences like this make little sense to me:

The function DrawAllShapes does not conform to the open-closed principle because it cannot be closed against new kinds of shapes.

Even Craig’s explanation feels somewhat at odds with Uncle Bob’s usage, as it talks about clients being affected. This is another of the issues I have with the original two descriptions: they talk about a single module being open/closed, whereas we’re dealing with abstractions where there are naturally at least two pieces of code involved (and usually three). Craig’s description of changes in one module not affecting clients is describing a relationship – which is a far more useful way of approaching things. Even thinking about the shape example, I’m getting increasingly confused about exactly what’s open and what’s closed. It feels to me like it’s neither the concrete shape classes nor the shape-drawing code which is open or closed – it’s the interface between the two; the abstract Shape class. After all, these statements seem reasonable:

  • The Shape class is open for extension: there can be many different concrete subclasses, and code which only depends on the Shape class doesn’t need to know about them in order to use them when they are presented as shapes.
  • The Shape class is closed for modification: no existing functions can be removed (as they may be relied on by existing clients) and no new pure virtual functions can be added (as they will not be implemented by existing subclasses).

It’s still not how I’d choose to express it, but at least it feels like it makes sense in very concrete terms. It doesn’t work well with how Uncle Bob uses the term "closed" though, so I still think I may be on a different page when it comes to that meaning. (Uncle Bob does also make the point that any significant program isn’t going to adhere to the principle completely in every part of the code – but in order to judge where it’s appropriate to be closed, I do really need to understand what being closed means.)

Just to make it crystal clear, other than the use of the word "closed," the low-level description of what’s good and what’s bad, and why, is absolutely fine. I really have no problems with it. As I said at the start, the idea being expressed makes perfect sense. It just doesn’t work (for me) when expressed in the terms used at a higher level.

Protected Variation

By contrast, let’s look at a closely related idea which I hadn’t actually heard about before I started all this research: protected variation. This name was apparently coined by Alistair Cockburn, and Craig Larman either quotes or paraphrases this as:

Identify points of predicted variation and create a stable interface around them.

Now that’s a description I can immediately identify with. Every single word of it makes sense to me, even without reading any more of Craig’s article. (I have read the rest, obviously, and I’d encourage others to do so.) This goes back to Josh Bloch’s "design for inheritance or prohibit it" motto: identifying points of predicted variation is hard, and it’s necessary in order to create a stable interface which is neither too constrictive for implementations nor too woolly for clients. With class inheritance there’s the additional concern of interactions within a class hierarchy when a virtual method is called.

So in Uncle Bob’s Shape example, all there is is a point of predicted variation: how the shape is drawn. PV suggests the converse as well – that as well as points of predicted variation, there may be points which will not vary. That’s inherent in the API to some extent – every shape must be capable of drawing itself with no further information (the Draw method has no parameters) but it could also be extended to non-virtual aspects. For example, we could decide that every shape has one (and only one) colour, which will not change during its lifetime. That can be implemented in the Shape class itself – with no predicted variation, there’s no need of polymorphism.

Of course, the costs of incorrectly predicting variation can be high: if you predict more variation than is actually warranted, you waste effort on over-engineering. If you predict less variation than is required, you usually end up either having to change quite a lot of code (if it’s all under your control) or having to come up with an "extended" interface. There’s the other aspect of shirking responsibility on this predicted variation to some extent, by making some parts "optional" – that’s like saying, "We know implementations will vary here in an incompatible way, but we’re not going to try to deal with it in the API. Good luck!" This usually arises in collection APIs, around mutating operations which may or may not be valid (based on whether the collection is mutable or not).

Not only is PV easy to understand – it’s easy to remember for its comedy value, at least if you’re a fan of The Hitchhiker’s Guide to the Galaxy. Remember Vroomfondel and Majikthise, the philosophers who invaded Cruxwan University just as Deep Thought was about to announce the answer to Life, the Universe, and Everything? Even though they were arguing with programmers, it sounds like they were actually the ones with software engineering experience:

"I’ll tell you what the problem is mate," said Majikthise, "demarcation, that’s the problem!"

[…]

"That’s right!" shouted Vroomfondel, "we demand rigidly defined areas of doubt and uncertainty!"

That sounds like a pretty good alternative description of Protected Variation to me.

Conclusion

So, that’s what I don’t like about OCP. The name, and the broad description – both of which I believe to be unhelpful, and poorly understood. (While I’ve obviously considered the possibility that I’m the only one who finds it confusing, I’ve heard enough variation in the explanations of it to suggest that I’m really not the only one.)

That sounds like a triviality, but I think it’s rather important. I suspect that OCP has been at least mentioned in passing in thousands if not tends of thousands of user groups and conferences. The purpose of such gatherings is largely for communication of ideas – and when a sound idea is poorly expressed, an opportunity is wasted. I suspect that any time Uncle Bob has personally presented it in detail, the idea has sunk in appropriately – possibly after some initial confusion about the terminology. But what about all the misinterpretations and "glancing blows" where OCP is only mentioned as a good thing that clearly everyone wants to adhere to, with no explanation beyond the obscure ones described in part one above? How many times did they shed more confusion than light?

I believe more people are familiar with Uncle Bob’s work on OCP than Bertrand Meyer’s. Further, I suspect that if Bertrand Meyer hadn’t already introduced the name and brief description, Uncle Bob may well have come up with far more descriptive ones himself, and the world would have been a better place. Fortunately, we do have a better name and description for a concept which is at least very closely related. (I’m not going to claim PV and OCP are identical, but close enough for a lot of uses.)

Ultimately, words matter – particularly when it comes to single sentence descriptions which act as soundbytes; shorthand for communicating a complex idea. It’s not about whether the more complex idea can be understood after carefully reading thorough explanations. It’s about whether the shorthand conveys the essence of the idea in a clear way. On that front, I believe the open-closed principle fails – which is why I’d love to see it retired in favour of more accessible ones.

Note for new readers

I suspect this post may end up being read more widely than most of my blog entries. If you’re going to leave a comment, please be aware that the CAPTCHA doesn’t work on Chrome. I’m aware of this, but can’t fix it myself. If you right-click on the broken image and select "open in new tab" you should get a working image. Apologies for the inconvenience.

Books, C#

C# in Depth 3rd edition available for early access, plus a discount code…

16 Comments

Readers who follow me on Twitter or Google+ know this already, but…

The third edition of C# in Depth is now available for early access from its page on the Manning website. I’ve been given a special discount code which expires at midnight EST on February 17th, so be quick if you want to use it – it gives 50% off either version. The code is “csharpsk”.

It’s likely that we’ll have a separate (permanent) discount for readers who already own the second edition, but the details of that haven’t been decided yet.

Just to be clear, the third edition is largely the second edition plus the changes to cover C# 5 – I haven’t done as much rewriting as I did for the second edition, mostly because I was already pretty happy with the second edition :) Obviously the largest (by far) feature in C# 5 is async/await, which is covered in detail in the new chapter 15.

C#

Fun with Object and Collection Initializers

12 Comments

Gosh it feels like a long time since I’ve blogged – particularly since I’ve blogged anything really C#-language-related.

At some point I want to blog about my two CodeMash 2013 sessions (making the C# compiler/team cry, and learning lessons about API design from the Spice Girls) but those will take significant time – so here’s a quick post about object and collection initializers instead. Two interesting little oddities…

Is it an object initializer? Is it a collection initializer? No, it’s a syntax error!

The first part came out of a real life situation – FakeDateTimeZoneSource, if you want to look at the complete context.

Basically, I have a class designed to help test time zone-sensitive code. As ever, I like to create immutable objects, so I have a builder class. That builder class has various properties which we’d like to be able to set, and we’d also like to be able to provide it with the time zones it supports, as simply as possible. For the zones-only use case (where the other properties can just be defaulted) I want to support code like this:

var source = new FakeDateTimeZoneSource.Builder
{
    CreateZone("x"),
    CreateZone("y"),
    CreateZone("a"),
    CreateZone("b")
}.Build();

(CreateZone is just a method to create an arbitrary time zone with the given name.)

To achieve this, I made the Builder implement IEnumerable<DateTimeZone>, and created an Add method. (In this case the IEnumerable<> implementation actually works; in another case I’ve used explicit interface implementation and made the GetEnumerator() method throw NotSupportedException, as it’s really not meant to be called in either case.)

So far, so good. The collection initializer worked perfectly as normal. But what about when we want to set some other properties? Without any time zones, that’s fine:

var source = new FakeDateTimeZoneSource.Builder
{
    VersionId = "foo"
}.Build();

But how could we set VersionId and add some zones? This doesn’t work:

var invalid = new FakeDateTimeZoneSource.Builder
{
    VersionId = "foo",
    CreateZone("x"),
    CreateZone("y")
}.Build();

That’s neither a valid object initializer (the second part doesn’t specify a field or property) nor a valid collection initializer (the first part does set a property).

In the end, I had to expose an IList<DateTimeZone> property:

var valid = new FakeDateTimeZoneSource.Builder
{
    VersionId = "foo",
    Zones = { CreateZone("x"), CreateZone("y") }
}.Build();

An alternative would have been to expose a propert of type Builder which just returned itself – the same code would have been valid, but it would have been distinctly odd, and allowed some really spurious code.

I’m happy with the result in terms of the flexibility for clients – but the class design feels a bit messy, and I wouldn’t have wanted to expose this for the "production" assembly of Noda Time.

Describing all of this to a colleague gave rise to the following rather sillier observation…

Is it an object initializer? Is it a collection initializer? (Parenthetically speaking…)

In a lot of C# code, an assignment expression is just a normal expression. That means there’s potentially room for ambiguity, in exactly the same kind of situation as above – when sometimes we want a collection initializer, and sometimes we want an object initializer. Consider this sample class:

using System;
using System.Collections;

class Weird : IEnumerable
{
    public string Foo { get; set; }
    
    private int count;
    public int Count { get { return count; } }
        
    public void Add(string x)
    {
        count++;
    }
            
    IEnumerator IEnumerable.GetEnumerator()
    {
        throw new NotSupportedException();
    }    
}

As you can see, it doesn’t actually remember anything passed to the Add method, but it does remember how many times we’ve called it.

Now let’s try using Weird in two ways which only differ in terms of parentheses. First up, no parentheses:

string Foo = "x";
Weird weird = new Weird { Foo = "y" };
    
Console.WriteLine(Foo);         // x
Console.WriteLine(weird.Foo);   // y
Console.WriteLine(weird.Count); // 0

Okay, so it’s odd having a local variable called Foo, but we’re basically fine. This is an object initializer, and it’s setting the Foo property within the new Weird instance. Now let’s add a pair of parentheses:

string Foo = "x";
Weird weird = new Weird { (Foo = "y") };
    
Console.WriteLine(Foo);         // y
Console.WriteLine(weird.Foo);   // Nothing (null)
Console.WriteLine(weird.Count); // 1

Just adding those parenthese turn the object initializer into a collection initializer, whose sole item is the result of the assignment operator – which is the value which has now been assigned to Foo.

Needless to say, I don’t recommend using this approach in real code…

Stack Overflow

Stack Overflow question checklist

37 Comments

Note: this post is now available with a tinyurl of http://tinyurl.com/stack-checklist

My earlier post on how to write a good question is pretty long, and I suspect that even when I refer people to it, often they don’t bother reading it. So here’s a short list of questions to check after you’ve written a question (and to think about before you write the question):

  • Have you done some research before asking the question? 1
  • Have you explained what you’ve already tried to solve your problem?
  • Have you specified which language and platform you’re using, including version number where relevant?
  • If your question includes code, have you written it as a short but complete program? 2
  • If your question includes code, have you checked that it’s correctly formatted? 3
  • If your code doesn’t compile, have you included the exact compiler error?
  • If your question doesn’t include code, are you sure it shouldn’t?
  • If your program throws an exception, have you included the exception, with both the message and the stack trace?
  • If your program produces different results to what you expected, have you stated what you expected, why you expected it, and the actual results?
  • If your question is related to anything locale-specific (languages, time zones) have you stated the relevant information about your system (e.g. your current time zone)?
  • Have you checked that your question looks reasonable in terms of formatting?
  • Have you checked the spelling and grammar to the best of your ability? 4
  • Have you read the whole question to yourself carefully, to make sure it makes sense and contains enough information for someone coming to it without any of the context that you already know?

    If the answer to any of these questions is “no” you should take the time to fix up your question before posting. I realize this may seem like a lot of effort, but it will help you to get a useful answer as quickly as possible. Don’t forget that you’re basically asking other people to help you out of the goodness of their heart – it’s up to you to do all you can to make that as simple as possible.

    1 If you went from “something’s not working” to “asking a question” in less than 10 minutes, you probably haven’t done enough research.

    2 Ideally anyone answering the question should be able to copy your code, paste it into a text editor, compile it, run it, and observe the problem. Console applications are good for this – unless your question is directly about a user interface aspect, prefer to write a short console app. Remove anything not directly related to your question, but keep it complete enough to run.

    3 Try to avoid code which makes users scroll horizontally. You may well need to change how you split lines from how you have it in your IDE. Take the time to make it as clear as possible for those trying to help you.

    4 I realize that English isn’t the first language for many Stack Overflow users. We’re not looking for perfection – just some effort. If you know your English isn’t good, see if a colleague or friend can help you with your question before you post it.

    • Noda Time

    Noda Time v1.0 released

    13 Comments

    Go get Noda Time 1.0!

    Today is the end of the longest release cycle I’ve been personally involved in. On November 5th 2009, I announced my intention to write a port of Joda Time for .NET. The next day, Noda Time was born – with a lofty (foolhardy) set of targets.

    Near the end of a talk *about* Noda Time this evening, I released Noda Time 1.0.0.

    It’s taken three years, but I’m immensely proud of what we’ve managed to achieve. We’re far from "done" but I believe we’re already significantly ahead of most other date/time APIs I’ve seen in terms of providing a clean API which reduces *incidental* complexity while highlighting the *inherent* complexity of the domain. (This is a theme I’m becoming dogmatic about on various fronts.)

    There’s more to do – I can’t see myself considering Noda Time to be "done" any time soon – but hopefully now we’ve got a stable release, we can start to build user momentum.

    One point I raised at the DotNetDevNet presentation tonight was that there’s a definite benefit (in my very biased view) in just *looking into* Noda Time:

    • If you can’t use it in your production code, use it when prototyping
    • If you can’t use it in your prototype code, play with it in personal projects
    • If you can’t use it in personal projects, read the user guide to understand the concepts

    I hope that simply looking at the various types that Noda Time providers will give you more insight into how you should be thinking about date and time handling in your code. While the BCL API has a lot of flaws, you can work around most of them if you make it crystal clear what your data means at every step. The type system will leave that largely ambiguous, but there’s nothing to stop you from naming your variables descriptively, and adding appropriate
    comments.

    Of course, I would far prefer it if you’d start using Noda Time and raising issues on how to make it better. Spread the word.

    Oh, and if anyone from the BCL team is reading this and would like to include something like Noda Time into .NET 5 as a "next generation" date/time, I’d be *really* interested in talking to you :)

    C#

    How can I enumerate thee? Let me count the ways…

    19 Comments

    This weekend, I was writing some demo code for the async chapter of C# in Depth – the idea was to decompile a simple asynchronous method and see what happened. I received quite a surprise during this, in a way which had nothing to do with asynchrony.

    Given that at execution time, text refers to an instance of System.String, and assuming nothing in the body of the loop captures the ch variable, how would you expect the following loop to be compiled?

    foreach (char ch in text)
    {
        // Body here
    }

    Before today, I could think of four answers depending on the compile-time type of text, assuming it compiled at all. One of those answers is if text is declared to be dynamic, which I’m not going to go into here. Let’s stick with static typing.

    If text is declared as IEnumerable…

    In this case, the compiler can only use the non-generic IEnumerator interface, and I’d expect the code to be roughly equivalent to this:

    IEnumerator iterator = text.GetEnumerator();
    try
    {
        while (iterator.MoveNext())
        {
            char ch = (char) iterator.Current;
            // Body here
        }
    }
    finally
    {
        IDisposable disposable = iterator as IDisposable;
        if (disposable != null)
        { 
            disposable.Dispose();
        }
    }

    Note how the disposal of the iterator has to be conditional, as IEnumerator doesn’t extend IDisposable.

    If text is declared as IEnumerable<char>…

    Here, we don’t need any execution time casting, and the disposal can be unconditional:

    IEnumerator<char> iterator = text.GetEnumerator();
    try
    {
        while (iterator.MoveNext())
        {
            char ch = iterator.Current;
            // Body here
        }
    }
    finally
    {
        iterator.Dispose();
    }

    If text is declared as string…

    Now things get interesting. System.String implements IEnumerable<char> using explicit interface implementation, and exposes a separate public GetEnumerator() method which is declared to return a CharEnumerator.

    Usually when I find a type doing this sort of thing, it’s for the sake of efficiency, to reduce heap allocations. For example, List<T>.GetEnumerator returns a List<T>.Enumerator which is struct with the appropriate iteration members. This means if you use foreach over an expression of type List<T>, the iterator can stay on the stack in most cases, saving object allocation and garbage collection.

    In this case, however, I suspect CharEnumerator was introduced (way back in .NET 1.0) to avoid having to box each character in the string. This was one reason for foreach handling to be based on types obeying the enumerable pattern, as well as there being support through the normal interfaces. It strikes me that it could still have been a structure in the same way as for List<T>, but maybe that wasn’t considered as an option.

    Anyway, it means that I would have expected the code to be compiled like this, even back to C# 1:

    CharEnumerator iterator = text.GetEnumerator();
    try
    {
        while (iterator.MoveNext())
        {
            char ch = iterator.Current;
            // Body here
        }
    }
    finally
    {
        iterator.Dispose();
    }

    What really happens when text is declared as string…

    (This is the bit that surprised me.)

    So far, I’ve been assuming that the C# compiler doesn’t have any special knowledge about strings, when it comes to iteration. I knew it did for arrays, but that’s all. The actual result – under the C# 5 compiler, at least – is to use the Length property and the indexer directly:

    int index = 0;
    while (index < text.Length)
    {
        char ch = text[index];
        index++;
        // Body here
    }

    There’s no heap allocation, and no Dispose call. If the variable in question can change its value within the loop (e.g. if it’s a field, or a captured variable, or there’s an assignment to it within the body) then a copy is made of the variable value (just a reference, of course) first, so that all member access is performed on the same object.

    Conclusion

    So, there we go. There’s nothing particularly mind-blowing here – certainly nothing to affect your programming style, unless you were deliberately avoiding using foreach on strings "because it’s slow." It’s still a good lesson in not assuming you know what the compiler is going to do though… so long as the results are as expected, I’m very happy for them to put extra smarts in, even if it does mean having to change my C# in Depth sample code a bit…

    General, Stack Overflow

    Stack Overflow and personal emails

    80 Comments

    This post is partly meant to be a general announcement, and partly meant to be something I can point people at in the future (rather than writing a short version of this on each email).

    These days, I get at least a few emails practically every day along the lines of:

    “I saw you on Stack Overflow, and would like you to answer this development question for me…”

    It’s clear that the author:

    • Is aware of Stack Overflow
    • Is aware that Stack Overflow is a site for development Q&A
    • Is aware that I answer questions on Stack Overflow

    … and yet they believe that the right way of getting me to answer a question is by emailing it to me directly. Sometimes it’s a link to a Stack Overflow question, sometimes it’s the question asked directly in email.

    In the early days of Stack Overflow, this wasn’t too bad. I’d get maybe one email like this a week. Nowadays, it’s simply too much.

    If you have a question worthy of Stack Overflow, ask it on Stack Overflow. If you’ve been banned from asking questions due to asking too many low-quality ones before, then I’m unlikely to enjoy answering your questions by email – learn what makes a good question instead, and edit your existing questions.

    If you’ve already asked the question on Stack Overflow, you should consider why you think it’s more worthy of my attention than everyone else’s questions. You should also consider what would happen if everyone who would like me to answer a question decided to email me.

    Of course in some cases it’s appropriate. If you’ve already asked a question, written it as well as you can, waited a while to see if you get any answers naturally, and if it’s in an area that you know I’m particularly experienced in (read: the C# language, basically) then that’s fine. If your question is about something from C# in Depth – a snippet which doesn’t work or some text you don’t understand, for example – then it’s entirely appropriate to mail me directly.

    Basically, ask yourself whether you think I will actually welcome the email. Is it about something you know I’m specifically interested in? Or are you just trying to get more attention to a question, somewhat like jumping a queue?

    I’m aware that it’s possible this post makes me look either like a grumpy curmudgeon or (worse) like an egocentric pseudo-celebrity. The truth is I’m just like everyone else, with very little time on my hands – time I’d like to spend as usefully and fairly as possible.

    Books, C#, C# 5

    The future of “C# in Depth”

    13 Comments

    I’m getting fairly frequent questions – mostly on Twitter – about whether there’s going to be a third edition of C# in Depth. I figure it’s worth answering it once in some detail rather than repeatedly in 140 characters ;)

    I’m currently writing a couple of new chapters covering the new features in C# 5 – primarily async, of course. The current "plan" is that these will be added to the existing 2nd edition to create a 3rd edition. There will be minimal changes to the existing text of the 2nd edition – basically going over the errata and editing a few places which ought to mention C# 5 early. (In particular the changes to how foreach loop variables are captured.)

    So there will definitely be new chapters. I’m hoping there’ll be a full new print (and ebook of course) edition, but no contracts have been signed yet. I’m hoping that the new chapters will be provided free electronically to anyone who’s already got the ebook of the 2nd edition – but we’ll see. Oh, and I don’t have any timelines at the moment. Work is more demanding than it was when I was writing the first and second editions, but obviously I’ll try to get the job done at a reasonable pace. (Writing about async in a way which is both accessible and accurate is really tricky, by the way.)

    Of course when I’ve finished those, I’ve got two other C# books I want to be writing… when I’m not working on Noda Time, Tekpub screencasts etc…

    Update

    I had a question on Twitter around the "two other C# books". I don’t want to go into too many details – partly because they’re very likely to change – but my intention is to write "C# from Scratch" and "C# in Style". The first would be for complete beginners; the second wouldn’t go into "how things work" so much as "how to use the language most effectively." (Yes, competition for Effective C#.) One possibility is that both would be donationware, at least in ebook form, ideally with community involvement in terms of public comments.

    I’m hoping that both will use the same codebase as an extended example, where "From Scratch" would explain what the code does, and "In Style" would explain why I chose that approach. Oh, and "From Scratch" would use unit testing as a teaching tool wherever possible, attempting to convey the idea that it’s something every self-respecting dev does :)

    C#, Design, Noda Time

    The perils of conditional mutability

    6 Comments

    This morning I was wrestling with trying to make some Noda Time unit tests faster. For some reason, the continuous integration host we’re using is really slow at loading resources under .NET 4. The unit tests which run in 10 seconds on my home laptop take over three hours on the continuous integration system. Taking stack traces at regular intervals showed the problem was with the NodaFormatInfo constructor, which reads some resources.

    I may look into streamlining the resource access later, but before we get to that point, I wanted to try to reduce the number of times we call that constructor in the first place. NodaFormatInfo is meant to be cached, so I wouldn’t have expected thousands of instances to be created – but it’s only cached when the System.Globalization.CultureInfo it’s based on is read-only. This is where the problems start…

    CultureInfo is conditionally mutable (not an official term, just one I’ve coined for the purposes of this post). You can ask whether or not it’s read-only with the IsReadOnly property, and obviously if it’s read-only you can’t change it. Additionally, CultureInfo is composed of other conditionally mutable objects – DateTimeFormatInfo, NumberFormatInfo etc. There’s a static ReadOnly method on CultureInfo to create a read-only wrapper around a mutable CultureInfo. It’s not clearly documented whether that’s expected to take a deep copy (so that callers can really rely on it not changing) or whether it’s expected to reflect any further changes made to the culture info it’s based on. To go in the other direction, you can call Clone on a CultureInfo to create a mutable copy of any existing culture.

    Further complications are introduced by the properties on the composite objects – we have properties such as DateTimeFormatInfo.MonthNames which returns a string array. Remember, arrays are always mutable. So it’s really important to know whether the array reference returned from the property refers to a copy of the underlying data, or whether it refers to the array that’s used internally by the type. Obviously for read-only DateTimeFormatInfo objects, I’d expect a copy to be returned – but for a mutable DateTimeFormatInfo, it would potentially make sense to return the underlying array reference. Unfortunately, the documentation doesn’t make this clear – but in practice, it always returns a copy. If you need to change the month names, you need to clone the array, mutate the clone, and then set the MonthNames property.

    All of this makes CultureInfo hard to work with. The caching decision earlier on only really works if a "read-only" culture genuinely won’t change behind the scenes. The type system gives you no help to catch subtle bugs at compile-time. Making any of this robust but efficient (in terms of taking minimal copies) is tricky to say the least.

    Not only does it make it hard to work with from a client’s point of view, but apparently it’s hard to implement correctly too…

    First bug: Mono’s invariant culture isn’t terribly invariant…

    (Broken in 2.10.8; apparently fixed later.)

    I discovered this while getting Noda Time’s unit tests to pass on Mono. Unfortunately there are some I’ve had to effectively disable at the moment, due to deficiencies in Mono (some of which are being fixed, of course).

    Here’s a program which builds a clone of the invariant culture, changes the clone’s genitive month names, and then prints out the first non-genitive name from the plain invariant culture:

    using System;
    using System.Globalization;

    class Test
    {
        static void Main()
        {        
            CultureInfo clone = (CultureInfo) CultureInfo.InvariantCulture.Clone();
            // Note: not even deliberately changing MonthNames for this culture!
            clone.DateTimeFormat.MonthGenitiveNames[0] = "Changed";
            
            // Prints Changed
            Console.WriteLine(CultureInfo.InvariantCulture.DateTimeFormat.MonthNames[0]);
        }
    }

    I believe this bug is really due to the lack of support for genitive month names in Mono at the moment – the MonthGenitiveNames property always just returns a reference to the month names for the invariant culture – without taking a copy first. (It always returns the invariant culture’s month names, even if you’re using a different culture entirely.) The code above shows an "innocent" attempt to change a mutable clone – but in reality we could have used any culture (including an immutable one) to make the change.

    Note that in the .NET implementation, the change would only have been to a copy of the underlying data, so even the clone wouldn’t have reflected change anywhere.

    Second bug: ReadOnly losing changes

    The second bug is the one I found this morning. It looks like it’s fixed in .NET 4, but it’s present in .NET 3.5, which is where it bit me this morning. When you try to make a read-only wrapper around a mutated culture, some of the properties are preserved… but some aren’t:

    using System;
    using System.Globalization;

    class Test
    {
        static void Main()
        {
            CultureInfo clone = (CultureInfo) CultureInfo.InvariantCulture.Clone();
            clone.DateTimeFormat.AMDesignator = "ChangedAm";

            // The array is recreated on each call to MonthNames, so changing the
            // value within the array itself doesn’t work :(
            string[] months = (string[]) clone.DateTimeFormat.MonthNames;
            months[0] = "ChangedMonth";
            clone.DateTimeFormat.MonthNames = months;
            
            CultureInfo readOnlyCopy = CultureInfo.ReadOnly(clone);
            Console.WriteLine(clone.DateTimeFormat.AMDesignator); // ChangedAm
            Console.WriteLine(clone.DateTimeFormat.MonthNames[0]); // ChangedMonth
                    
            Console.WriteLine(readOnlyCopy.DateTimeFormat.AMDesignator); // ChangedAm
            Console.WriteLine(readOnlyCopy.DateTimeFormat.MonthNames[0]); // January (!)
        }
    }

    I don’t know what’s going on here. In the end I just left the test code using the mutable clone, having added a comment explaining why it wasn’t created a read-only wrapper.

    I’ve experimented with a few different options here, including setting the MonthNames property on the clone after creating the wrapper. No joy – I simply can’t make the new month names stick in the read-only copy. <sigh>

    Conclusion

    I’ve been frustrated by the approach we’ve taken to cultures in Noda Time for a while. I haven’t worked out exactly what we should do about it yet, so it’s likely to stay somewhat annoying for v1, but I may well revisit it significantly for v2. Unfortunately, there’s nothing I can do about CultureInfo itself.

    What I would have preferred in all of this is the builder pattern: make CultureInfo, DateTimeFormatInfo etc all immutable, but give each of them mutable builder types, with the ability to create a mutable builder based on an existing immutable instance, and obviously to create a new immutable instance from a builder. That would make all kinds of things simpler – including caching.

    For the moment though, I hope we can all learn lessons from this – or have old lessons reinforced, at least:

    • Making a single type behave in different ways based on different "modes" makes it hard to use correctly. (Yes, this is the same first conclusion as with DateTime in the previous post. Funny, that.)
    • Immutability has to be deep to be meaningful: it’s not much use having a supposedly read-only object which composes a StringBuilder…
    • Arrays should be considered somewhat harmful. If you’re going to return an array from a method, make sure you document whether this is a copy of the underlying data, or a "live" reference. (The latter should be very rare, particularly for a public API.) The exception here is if you return an empty array – that’s effectively immutable, so you can always return it with no problems.
    • The builder pattern rocks – use it!

    In my next post I’ll try to get back to the TimeZoneInfo oddities I mentioned last time.

    C#, Evil code, Noda Time

    More fun with DateTime

    20 Comments

    (Note that this is deliberately not posted in the Noda Time blog. I reckon it’s of wider interest from a design perspective, and I won’t be posting any of the equivalent Noda Time code. I’ll just say now that we don’t have this sort of craziness in Noda Time, and leave it at that…)

    A few weeks ago, I was answering a Stack Overflow question when I noticed an operation around dates and times which should have been losing information apparently not doing so. I investigated further, and discovered some "interesting" aspects of both DateTime and TimeZoneInfo. In an effort to keep this post down to a readable length (at least for most readers; certain WebDriver developers who shall remain nameless have probably given up by now already) I’ll save the TimeZoneInfo bits for another post.

    Background: daylight saving transitions and ambiguous times

    There’s one piece of inherent date/time complexity you’ll need to understand for this post to make sense: sometimes, a local date/time occurs twice. For the purposes of this post, I’m going to assume you’re in the UK time zone. On October 28th 2012, at 2am local time (1am UTC), UK clocks will go back to 1am local time. So 1:20am local time occurs twice – once at 12:20am UTC (in daylight saving time, BST), and once at 1:20am UTC (in standard time, GMT).

    If you want to run any of the code in this post and you’re not in the UK, please adjust the dates and times used to a similar ambiguity for when your clocks go back. If you happen to be in a time zone which doesn’t observe daylight savings, I’m afraid you’ll have to adjust your system time zone in order to see the effect for yourself.

    DateTime.Kind and conversions

    As you may already know, as of .NET 2.0, DateTime has a Kind property, of type DateTimeKind – an enum with the following values:

    • Local: The DateTime is considered to be in the system time zone. Not an arbitrary "local time in some time zone", but in the specific current system time zone.
    • Utc: The DateTime is considered to be in UTC (corollary: it always unambiguously represents an instant in time)
    • Unspecified: This means different things in different contexts, but it’s a sort of "don’t know" kind; this is closer to "local time in some time zone" which is represented as LocalDateTime in Noda Time.

    DateTime provides three methods to convert between the kinds:

    • ToUniversalTime: if the original kind is Local or Unspecified, convert it from local time to universal time in the system time zone. If the original kind is Utc, this is a no-op.
    • ToLocalTime: if the original kind is Utc or Unspecified, convert it from UTC to local time. If the original kind is Local, this is a no-op.
    • SpecifyKind: keep the existing date/time, but just change the kind. (So 7am stays as 7am, but it changes the meaning of that 7am effectively.)

    (Prior to .NET 2.0, ToUniversalTime and ToLocalTime were already present, but always assumed the original value needed conversion – so if you called x.ToLocalTime().ToLocalTime().ToLocalTime() the result would probably end up with the appropriate offset from UTC being applied three times!)

    Of course, none of these methods change the existing value – DateTime is immutable, and a value type – instead, they return a new value.

    DateTime’s Deep Dark Secret

    (The code in this section is presented in several chunks, but it forms a single complete piece of code – later chunks refer to variables in earlier chunks. Put it all together in a Main method to run it.)

    Armed with the information in the previous sections, we should be able to make DateTime lose data. If we start with 12:20am UTC and 1:20am UTC on October 28th as DateTimes with a kind of Utc, when we convert them to local time (on a system in the UK time zone) we should get 1:20am in both cases due to the daylight saving transition. Indeed, that works:

    // Start with different UTC values around a DST transition
    var original1 = new DateTime(2012, 10, 28, 0, 20, 0, DateTimeKind.Utc);
    var original2 = new DateTime(2012, 10, 28, 1, 20, 0, DateTimeKind.Utc);

    // Convert to local time
    var local1 = original1.ToLocalTime();
    var local2 = original2.ToLocalTime();

    // Result is the same for both values. Information loss?
    var expected = new DateTime(2012, 10, 28, 1, 20, 0, DateTimeKind.Local);
    Console.WriteLine(local1 == expected); // True
    Console.WriteLine(local2 == expected); // True
    Console.WriteLine(local1 == local2);   // True

    If we’ve started with two different values, applied the same operation to both, and ended up with equal values, then we must have lost information, right? That doesn’t mean that operation is "bad" any more than "dividing by 2" is bad. You ought to be aware of that information loss, that’s all.

    So, we ought to be able to demonstrate that information loss further by converting back from local time to universal time. Here we have the opposite problem: from our local time of 1:20am, we have two valid universal times we could convert to – either 12:20am UTC or 1:20am UTC. Both answers would be correct – they are universal times at which the local time would be 1:20am. So which one will get picked? Well… here’s the surprising bit:

    // Convert back to UTC
    var roundTrip1 = local1.ToUniversalTime(); 
    var roundTrip2 = local2.ToUniversalTime();

    // Values round-trip correctly! Information has been recovered…
    Console.WriteLine(roundTrip1 == original1);  // True
    Console.WriteLine(roundTrip2 == original2);  // True
    Console.WriteLine(roundTrip1 == roundTrip2); // False

    Somehow, each of the local values knows which universal value it came from. The The information has been recovered, so the reverse conversion round-trips each value back to its original one. How is that possible?

    It turns out that DateTime actually has four potential kinds: Local, Utc, Unspecified, and "local but treat it as the earlier option when resolving ambiguity". A DateTime is really just a 64-bit number of ticks, but because the range of DateTime is only January 1st 0001 to December 31st 9999. That range can be represented in 62 bits, leaving 2 bits "spare" to represent the kind. 2 bits gives 4 possible values… the three documented ones and the shadowy extra one.

    Through experimentation, I’ve discovered that the kind is preserved if you perform arithmetic on the value, too… so if you go to another "fall back" DST transition such as October 30th 2011, the ambiguity resolution works the same way as before:

    var local3 = local1.AddYears(-1).AddDays(2); 
    var local4 = local2.AddYears(-1).AddDays(2);        
    Console.WriteLine(local3.ToUniversalTime().Hour); // 0
    Console.WriteLine(local4.ToUniversalTime().Hour); // 1

    If you use DateTime.SpecifyKind with DateTimeKind.Local, however, it goes back to the "normal" kind, even though it looks like it should be a no-op:

    // Should be a no-op?
    var local5 = DateTime.SpecifyKind(local1, local1.Kind); 
    Console.WriteLine(local5.ToUniversalTime().Hour); // 1

    Is this correct behaviour? Or should it be a no-op, just like calling ToLocalTime on a "local" DateTime is? (Yes, I’ve checked – that doesn’t lose the information.) It’s hard to say, really, as this whole business appears to be undocumented… at least, I haven’t seen anything in MSDN about it. (Please add a link in the comments if you find something. The behaviour actually goes against what’s documented, as far as I can tell.)

    I haven’t looked into whether various forms of serialization preserve values like this faithfully, by the way – but you’d have to work hard to reproduce it in non-framework code. You can’t explicitly construct a DateTime with the "extra" kind; the only ways I know of to create such a value are via a conversion to local time or through arithmetic on a value which already has the kind. (Admittedly if you’re serializing a DateTime with a Kind of Local, you’re already on potentially shaky ground, given that you could be deserializing it on a machine with a different system time zone.)

    Unkind comparisons

    I’ve misled you a little, I have to admit. In the code above, when I compared the "expected" value with the results of the first conversions, I deliberately specified DateTimeKind.Local in the constructor call. After all, that’s the kind we do expect. Well, yes – but I then printed the result of comparing this value with local1 and local2… and those comparisons would have been the same regardless of the kind I’d specified in the constructor.

    All comparisons between DateTimes ignore the Kind property. It’s not just restricted to equality. So for example, consider this comparison:

    // In June: Local time is UTC+1, so 8am UTC is 9am local
    var dt1 = new DateTime(2012, 6, 1, 8, 0, 0, DateTimeKind.Utc); 
    var dt2 = new DateTime(2012, 6, 1, 8, 30, 0, DateTimeKind.Local); 
    Console.WriteLine(dt1 < dt2); // True

    When viewed in terms of "what instants in time do these both represent?" the answer here is wrong – when you convert both values into the same time zone (in either direction), dt1 occurs after dt2. But a simple look at the properties tells a different story. In practice, I suspect that most comparisons between DateTime values of different kinds involve code which is at best sloppy and is quite possibly broken in a meaningful way.

    Of course, if you bring Kind=Unspecified into the picture, it becomes impossible to compare meaningfully in a kind-sensitive way. Is 12am UTC before or after 1am Unspecified? It depends what time zone you later use.

    To be clear, it is a hard-to-resolve issue, and one that we don’t do terribly well at in Noda Time at the moment for ZonedDateTime. (And even with just LocalDateTime you’ve got issues between calendars.) This is a situation where providing separate Comparer<T> implementations works nicely – so you can explicitly say what kind of comparison you want.

    Conclusions

    There’s more fun to be had with a similar situation when we look at TimeZoneInfo, but for now, a few lessons:

    • Giving a type different "modes" which make it mean fairly significantly different things is likely to cause headaches
    • Keeping one of those modes secret (and preventing users from even constructing a value in that mode directly) leads to even more fun and games
    • If two instances of your type are considered "equal" but behave differently, you should at least consider whether there’s something smelly going on
    • There’s always more fun to be had with DateTime…