Is using a constant value as a "nil" (absence of value) a good practice in this case?

Question

I'm learning Swift and I'm facing a problem in one of my model classes. What I'm trying to do is having a lazy-loaded property that can be "invalidated" when the data it is based on changes. (something like this: https://stackoverflow.com/a/25954243/2382892)

What I have now is something like this:

class DataSet {
    var entries: [Entry]

    var average: Double? {
        return self.entries.average
    }
}

Array<Entry>.average computes the average of a property of Entry, but returns nil if the array is empty.

Since this average could potentially be expensive to compute, I'd like to do that lazily and store a cached value of it and recalculate it only if necessary (when DataSet.entries is modified).

Now, following this answer, I should do something like this:

class DataSet {
    var entries: [Entry]
    var _average: Double?
    var average: Double? {
        if _average == nil {
            _average = self.entries.average
        }
        return _average
    }
}

However I have the problem of handling both the case that the average is to be recomputed, and when the array is empty and there's no meaningful average to return.

Since I know the average value will always be positive, I could use a default value (such as -1.0) to indicate the cache is no more valid, and nil to mean there are no entries, or vice-versa (nil means the average has to be computed again; -1.0 when there are no entries).

This however doesn't seem elegant, or the "Swift way" to achieve this behavior (or is it indeed?).

What should I do?

Answer 1

You definitely shouldn't use magic values, like -1 to indicate some state. But I agree that you should not use nil to indicate both that "the cached value is invalidated and must be recalculated" as well as "the cached average has been calculated and is nil because there were zero Entry objects". The problem with the other solutions that suggest just setting the calculated value to nil is that it can't distinguish between the two states of "invalidated" and "calculated but nil" and may call entries.average even though you may have already done so. Admittedly, this is likely not computationally expensive, but it is conflating two very different states.

One Swift solution would be an enum:

class DataSet {
    private enum CachedValue {
        case Calculated(Double?)
        case Invalidated
    }

    var entries = [Entry]() {
        didSet {
            cachedAverage = .Invalidated
        }
    }

    private var cachedAverage: CachedValue = .Invalidated

    var average: Double? {
        switch cachedAverage {
        case .Calculated(let result):
            return result
        case .Invalidated:
            let result = entries.average
            cachedAverage = .Calculated(result)
            return result
        }
    }
}

This captures a distinct state between .Invalidated and .Calculated and recalculates lazily as needed.

Answer 2

Use nil

First of all never use a specific value of your type domain to indicate the absence of value. In other words don't use a negative number to indicate no value. nil is the answer here.

So average should be declared as Double?.

Clearing the cache when entries does change

Next you need to clear your cache each time entries is mutated. You can use didSet for this.

class DataSet {
    private var entries: [Entry] = [] {
        didSet { cachedAverage = nil }
    }

    private var cachedAverage: Double?
    var average: Double? {
        if cachedAverage == nil {
            cachedAverage = self.entries.average
        }
        return cachedAverage
    }
}

When entries is empty

Finally if you believe that average for an empty array should be nil, then why don't you change accordingly the average computed property you added to SequenceType?

Answer 3

You can make use of didSet:

class DataSet {
    var entries: [Entry] {
        didSet {
            /// just mark the average as outdated, it will be
            /// recomputed when someone asks again for it
            averageOutOfDate = true
        }
    }

    /// tells us if we should recompute, by default true
    var averageOutOfDate = true
    /// cached value that avoid the expensive computation
    var cachedAverage: Double? = nil

    var average: Double? {
        if averageOutOfDate {
            cachedAverage = self.entries.average
            averageOutOfDate = false
        }
        return cachedAverage
    }
}

Basically whenever the entries property value changes, you mark the cached value as outdated, and you use this flag to know when to recompute it.

Answer 4

appzYourLife's answer is a nice concrete solution. However, I suggest a differ approach all-together. Here's what I would do:

I would make a protocol that defines all the important bits that would need to be access externally.

I would then make 2 structs/classes conform to this protocol.

The first struct/class will be a caching layer.

The second struct/class will be the actual implementation, which will only ever be accessed by the caching layer.

The caching layer will have a private instance of the actual implementing struct/class, and will have a variable such as "isCacheValid", which will be set to false by all mutating operations that invalidate the underlying data (and by extension, computed values such as the average).

This design makes it so that the actual implementing struct/class is fairly simple, and completely agnostic to the caching.

The caching layer does all the caching duties, completely agnostic to details of how the cached values are computed (as their computation is just delegated to the implementing class.