Coding Style – Defensive Copying Patterns

coding-style

While watching a youtube video on Value Types in Swift, I was surprised by a simple example (around the 3:00 minute mark) that was given to demonstrate the pitfalls of reference types.

Example code:

let home = House()
let temp = Temperature()
temp.fahrenheit = 75
home.thermostat.temperature = temp.copy()

temp.fahrenheit = 425
home.oven.temperature = temp.copy()
home.oven.bake()

In the above code, House and Temperature are classes and therefore have reference type semantics. The justification for invoking the copy() method in several places is to ensure that mutating the temp variable for setting the oven doesn't modify the temperature of the house's thermostat (i.e. prevent unintended mutation due to reference semantics).

I personally find this a very strange coding style, but the presenter goes on for several minutes about how this so-called "defensive copying" is used throughout Apple's core librairies to prevent bugs so I'm wondering if I've been doing things wrong.

Personally, I would have written this along the following lines:

let home = House()
home.thermostat.temperature = Temperature()
home.thermostat.temperature.fahrenheit = 75   //actually I would personally have supplied this as a parameter to the initializer but I'm trying to remain as close to the original code as possible.

home.oven.temperature = Temperature()
home.oven.temperature.fahrenheit = 425
home.oven.bake()

In other words, I would explicitly create new instances of the Temperature class by calling its initializer (constructor) instead of reusing a local variable of that type and assigning copies of it. Not only does it avoid the last copy, which is useless, but the code seems clearer to me.

Now granted, this is a trivial example. In the case of a complex class with many members that share the same values between instances, recreating a new type from scratch and assigning every value over and over is a lot of programming overhead. However, in my experience at least, I've usually created initializers or factory methods that perform these common assignments so I only have to make a few changes where needed.

In the rare cases where the above doesn't apply, I find that explicitly calling a clone() or copy() method is fine. By only doing so rarely, it conveys the fact to me that something special is going on (e.g. deep copy of a tree for example).

Therefore, my question is: is Apple's above code a recommended practice? And if not, in what circumstances (other than deep copies) would manual copying as above be the preferred way of doing things?

Best Answer

I agree with Phil Lello that this has probably been over-simplified, possibly to the point that its confusing. A much clearer example of the need and undesirability of (needing to do) defensive copying is the following using C# for concreteness.

class House {
    private Thermostat thermostat;
    private ControlDisplay cc;
    // ...
    public void ShowDisplay() {
        // ...
        cd.SetTemperatureField(thermostat.Temperature.Clone());
        // ...
        cd.UpdateDisplay();
    }
}

Now, if Temperature is mutable, every time we want to show the control display we need to copy the thermostat's temperature even though it is unlikely to be changed by the display. Even if we control the code for ControlDisplay and thus can verify by looking at the source code that it doesn't mutate the passed in Temperature, we still copy because that may be changed in the future. There was no way to communicate and enforce that it shouldn't be changed.

If Temperature were immutable, on the other hand, there would be no need for this copying. The result would be cleaner, more efficient code, that simply makes certain mistakes and couplings impossible.

I imagine most of the instances in the Apple's core libraries are more like the above than like the example provided in the video. I can only imagine a few, somewhat odd, scenarios where code like the original example might conceivably be preferable to code more like your second example (or your alluded to rewriting).

Related Topic