Should a getter be allowed to return a different value than was passed to the setter?

Question

For example, given the following class:

public class SomeClass
{
    //...

    private IEnumerable<SomeType> myEnumerable;

    public IEnumerable<SomeType> MyEnumerable
    {
        get => myEnumerable;
        set => myEnumerable = value ?? new List<SomeType>();
    }
}

Is it good practice to perform this type of coercion in the setter (or in the getter), or is it more important not to have the property behave differently than a field - for example, in the following scenario:

var obj = new SomeClass();
IEnumerable<SomeType> enumerable = GetEnumerable(); //returns null
obj.MyEnumerable = enumerable;
Assert.AreEqual(enumerable, obj.MyEnumerable); //wrong!  

Answer 1

You have a few combined questions here.

• Is a setter allowed to store a different value than it received?

Yes, if it is warranted. However, for a reference type, you should really be storing the default value instead of merely returning it. Otherwise, it may create unexpected behavior.

• Is a getter allowed to return a different value than is stored?

Yes, if it is warranted.

• Is this current example a situation where you need to do this?

No. In this example, it's a bad workaround solution to a completely different problem.

---

Why you shouldn't do it in this example.

The issue with your code is that the solution is implemented in a different place than the problem, rendering your code disjointed.

public IEnumerable<SomeType> MyEnumerable
{
    get => myEnumerable;
    set => myEnumerable = value ?? new List<SomeType>();
}

The "custom" piece of logic here is ?? new List<SomeType>(). Everything else is a default implementation of a property. This code is the logical equivalent of "we need to convert null values into a default value".

If we need to convert null values into a default value; the first question you should ask yourself is where the null value is coming from, which is here:

IEnumerable<SomeType> enumerable = GetEnumerable(); //returns null

And now we see the disjointed problem/solution:

• The problem occurs inside the GetEnumerable() method.
• The solution occurs inside the property of a class which is used to store the GetEnumerable() return value.

This is problematic, for several reasons:

• If GetEnumerable() is used in other locations, and you're storing the value in SomeOtherClass, then this class needs to also handle potential null values. This violates the DRY principle.
• It also unnecessarily ties SomeClass to the GetEnumerable() method, since it contains validation that is (fairly) specific to the output of the GetEnumerable() method. This violates the separation of concerns.

---

The best solutions, in order of preference.

1. Change the GetEnumerable() method.

Ideally, a method should never return null. There are exceptions to this, but they are not the case for your example.
E.g. a method like SingleOrDefault() somewhat explicitly specifies that null is a meaningful return value. By using SingleOrDefault() instead of Single(), the developer has explicitly specified that he'd rather have a null (or defined default) value instead of an exception. The developer is therefore aware that he may encounter null values (unless he specified a different default value).

This is simply fixed in the GetEnumerable() method body. If the return statement is:

return myVariable;

it can simply be changed to:

return myVariable ?? new List<SomeType>();

Problem solved, no null values will be returned.

However, it's possible that you have no access to the method itself, e.g. if this is a method from an external library.

2. Immediately handle any returned null.

Simply put, you inline the method call and the null-handling:

IEnumerable<SomeType> enumerable = GetEnumerable() ?? new List<SomeType>();

While the method can still return null, which you have no control over, you immediately convert that null into an empty list. All your subsequent code is no longer required to assess whether or not a null value might have been passed.

Side comment:
If this method is used in many different locations, it's worth it to wrap the method in another method which handles the null values:

public List<SomeType> GetEnumerableOrDefault()
{
     return GetEnumerable() ?? new List<SomeType>();
}

This way, you don't have to put ?? new List<SomeType>() every time you want to call GetEnumerable().

---

Other cases where you would want to change the getter/setter value:

This is just to give you a better use cases where you'd want to meddle with the values:

• Preventing illogical values, e.g. a negative age:

e.g.

public int Age
{
    get => _age;
    set => _ age = value < 0 ? 0 : value;
}

Note: By modern day standards, this is considered putting business logic into a property; which is frowned upon. However, arguments can be made for simple applications to use such an approach.

• Preventing null references in case the property has not been initialized:

e.g.

public string Name
{
    get => _name ?? "John Doe";
    set => _name = value;
}

Note:

• I'm struggling to find a good reason to use this. If it's a value type, null is generally not an issue. If it's a reference type, it's dangerous to not store the default object in the field. Strings could possibly be an exception to this, but for string, you'll often want to check for empty values as well. So I can't think of a good use case right now, other than nullable value types (which is not all that common).
• More often than not, the property should either already be initialized in the constructor or by initializing it. Generally, the issue lies with whoever decided to set a null value, as opposed to the unlucky caller who happens to receive a null value after the fact.
• For strings, a more sensible approach would be to check for empty strings as well: get => String.IsNullOrWhitespace(_name) ? "John Doe" : _name;.
• Do not use this for reference types. You'll end up causing unexpected behavior for an external caller, as you pretend that the value is inside the property, but behind the scenes you never stored it.

Answer 2

To me, it would seem confusing for a developer to write

someObject.List = null;
someObject.Add(item);

It would make more sense for that sort of thing to throw an exception. The developer ought to write

someObject.List = new List<SomeType>();
someObject.List.Add(item);

Or better yet,

someObject.List.Clear();
someObject.List.Add(item);

So I guess a more standard way to implement your property would be like this:

public class SomeClass
{
    private IEnumerable<SomeType> myEnumerable = new List<SomeType>();

    public IEnumerable<SomeType> MyEnumerable
    {
        get => myEnumerable;
        set => {
                   if (value == null) throw new ExceptionOfSomeKind();
                   myEnumerable = value;
               }
    }
}

Or this:

public class SomeClass
{
    //This is readonly now-- can be set only when instantiated
    private readonly List<SomeType> myEnumerable = new List<SomeType>();

    public List<SomeType> MyList
    {
        get => myEnumerable;
        //Notice no setter!!!
    }
}

Answer 3

The entire reason to uses properties with getters and setters is exactly because you don't always want to return what's passed to it.

Otherwise why would you use anything else then a field?