25

I have a class with two readonly int fields. They are exposed as properties:

public class Thing
{
    private readonly int _foo, _bar;

    /// <summary> I AM IMMUTABLE. </summary>
    public Thing(int foo, int bar)
    {
        _foo = foo;
        _bar = bar;
    }

    public int Foo { get { return _foo; } set { } }

    public int Bar { get { return _bar; } set { } }
}

However, that means that the following is perfectly legal code:

Thing iThoughtThisWasMutable = new Thing(1, 42);

iThoughtThisWasMutable.Foo = 99;  // <-- Poor, mistaken developer.
                                  //     He surely has bugs now. :-(

The bugs that come from assuming that would work are sure to be insidious. Sure, the mistaken developer should have read the docs. But that doesn't change the fact that no compile- or run-time error warned him about the problem.

How should the Thing class be changed so that devs are less likely to make the above mistake?

Throw an exception? Use a getter method instead of a property?

  • 6
  • 1
    Thanks, @gnat. That post (both the question and the answer) seems to be talking about interfaces, as in Capital I Interfaces. I'm not sure that's what I'm doing. – kdbanman Jul 21 '15 at 17:46
  • 6
    @gnat, that is dreadful advice. Interfaces offer a great way of serving up publicly immutable VOs/DTOs that are still easy to test. – David Arno Jul 21 '15 at 17:53
  • 4
    @gnat, I did and the question makes no sense as the OP seems to think that interfaces will destroy immutability, which is nonsense. – David Arno Jul 21 '15 at 17:56
  • 1
    @gnat, that question was about declaring interfaces for DTOs. The top voted answer was that interfaces would not be harmful, but probably unnecessary. – Paul Draper Jul 22 '15 at 5:05
107

Why make that code legal?
Take out the set { } if it does nothing.
This is how you define a read only public property:

public int Foo { get { return _foo; } }
  • 3
    I didn't think that was legal for some reason, but it seems to work perfectly! Perfect, thank you. – kdbanman Jul 21 '15 at 17:49
  • 1
    @kdbanman It probably has something to do with something you saw the IDE doing at one point - probably auto-completion. – Panzercrisis Jul 22 '15 at 21:35
  • 2
    @kdbanman; what is not legal (upto C#5) is public int Foo { get; } (auto-property with only a getter) But public int Foo { get; private set; } is. – Laurent LA RIZZA Jul 23 '15 at 14:08
  • @LaurentLARIZZA, you've jogged my memory. That's exactly where my confusion came from. – kdbanman Jul 23 '15 at 15:05
45

With C#5 and before, we were faced with two options for immutable fields exposed via a getter:

  1. Create a read-only backing variable and return that via a manual getter. This option is secure (one must explicitly remove the readonly to destroy the immutability. It created lots of boiler-plate code though.
  2. Use an auto-property, with a private setter. This creates simpler code, but it's easier to accidentally break the immutability.

With C# 6 though (available in VS2015, which was released yesterday), we now get the best of both worlds: read-only auto properties. This allows us to simplify the OP's class to:

public class Thing
{
    /// <summary> I AM IMMUTABLE. </summary>
    public Thing(int foo, int bar)
    {
        Foo = foo;
        Bar = bar;
    }

    public int Foo { get; }
    public int Bar { get; }
}

The Foo = foo and Bar = bar lines are only valid in the constructor (which achieves the robust read-only requirement) and the backing field is implied, rather than having to be explicitly defined (which achieves the simpler code).

12

You could just get rid of the setters. They don't do anything, they will confuse users and they will lead to bugs. However, you could instead make them private and thus get rid of the backing variables, simplifying your code:

public class Thing
{
    public Thing(int foo, int bar)
    {
        Foo = foo;
        Bar = bar;
    }

    public int Foo { get; private set; }

    public int Bar { get; private set; }
}
  • 1
    "public int Foo { get; private set; }" That class is no longer immutable because it can change itself. Thanks, though! – kdbanman Jul 21 '15 at 17:47
  • 4
    The class described is immutable, as it doesn't change itself. – David Arno Jul 21 '15 at 17:48
  • 1
    My actual class is a bit more complex that the MWE I gave. I'm after real immutability, complete with thread safety and class-internal guarantees. – kdbanman Jul 21 '15 at 18:01
  • 4
    @kdbanman, and your point is? If your class only writes to the private setters during construction, then it has implemented "real immutability". The readonly keyword is just a poka-yoke, ie it guards against the class breaking the immutablity in future; it isn't needed to achieve immutability though. – David Arno Jul 21 '15 at 18:09
  • 3
    Interesting term, I had to google it - poka-yoke is a Japanese term that means "mistake-proofing". You're right! That's exactly what I'm doing. – kdbanman Jul 21 '15 at 18:14
6

Two solutions.

Simple:

Don't include setters as noted by David for immutable readonly objects.

Alternatively:

Allow setters to return a new immutable object, verbose in comparison to the former but provides state over time for each initialised object. This design is a very useful tool for Thread safety and immutability which extends over all imperative OOP languages.

Pseudocode

public class Thing
{

{readonly vars}

    public Thing(int foo, int bar)
    {
        _foo = foo;
        _bar = bar;
    }

    public Thing withFoo(int foo) {
        return new Thing(foo, getBar());
    }

    public Thing withBar(int bar) {
        return new Thing(getFoo(), bar)
    }

    etc...
}

public static factory

public class Thing
{

{readonly vars}

    private Thing(int foo, int bar)
    {
        _foo = foo;
        _bar = bar;
    }

    public static with(int foo, int bar) {
        return new Thing(foo, bar)
    }

    public Thing withFoo(int foo) {
        return new Thing(foo, getBar());
    }

    public Thing withBar(int bar) {
        return new Thing(getFoo(), bar)
    }

    etc...
}
  • 2
    setXYZ are horrible names for factory methods, consider withXYZ or the like. – Ben Voigt Jul 22 '15 at 14:34
  • Thanks, updated my answer to reflect your helpful comment. :-) – Matt Smithies Jul 22 '15 at 14:37
  • The question was asking specifically about property setters, not setter methods. – immibis Jul 23 '15 at 1:19
  • True, but the OP is worried about mutable state from a possible future developer. Variables using private readonly or private final keywords should be self documenting, that isn't the original devs fault if that isn't understood. Understanding different methods to change state is key. I have added another approach which is very helpful. There are alot of things in code world that cause excessive paranoia, private readonly vars shouldn't be one of them. – Matt Smithies Jul 23 '15 at 7:50

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.