13

I am having a design issue regarding .NET properties.

interface IX
{
    Guid Id { get; }
    bool IsInvalidated { get; }
    void Invalidate();
}

Problem:

This interface has two read-only properties, Id and IsInvalidated. The fact that they are read-only, however, is by itself no guarantee that their values will remain constant.

Let's say that it were my intention to make it very clear that…

  • Id represents a constant value (which may therefore be safely cached), while
  • IsInvalidated might change its value during the lifetime of an IX object (and so shouldn't be cached).

How could I modify interface IX to make that contract sufficiently explicit?

My own three attempts at a solution:

  1. The interface is already well-designed. The presence of a method called Invalidate() allows a programmer to infer that the value of the similarly-named property IsInvalidated might be affected by it.

    This argument holds only in cases where the method and property are similarly named.

  2. Augment this interface with an event IsInvalidatedChanged:

    bool IsInvalidated { get; }
    event EventHandler IsInvalidatedChanged;
    

    The presence of a …Changed event for IsInvalidated states that this property may change its value, and the absence of a similar event for Id is a promise that that property will not change its value.

    I like this solution, but it's a lot of additional stuff that may not get used at all.

  3. Replace the property IsInvalidated with a method IsInvalidated():

    bool IsInvalidated();
    

    This might be too subtle a change. It's supposed to be a hint that a value is computed freshly each time — which wouldn't be necessary if it was a constant. The MSDN topic "Choosing Between Properties and Methods" has this to say about it:

    Do use a method, rather than a property, in the following situations. […] The operation returns a different result each time it is called, even if the parameters do not change.

What kind of answers do I expect?

  • I am most interested in entirely different solutions to the problem, along with an explanation how they beat my above attempts.

  • If my attempts are logically flawed or have significant disadvantages not yet mentioned, such that only one solution (or none) remains, I would like to hear about where I went wrong.

    If the flaws are minor, and more than one solution remains after taking it into consideration, please comment.

  • At the very least, I would like some feedback on which is your preferred solution, and for what reason(s).

stakx
  • 2,128

4 Answers4

8

There is a fourth solution: rely on documentation.

How do you know that string class is immutable? You simply know that, because you've read the MSDN documentation. StringBuilder, on the other hand, is mutable, because, again, documentation tell you that.

/// <summary>
/// Represents an index of an element stored in the database.
/// </summary>
private interface IX
{
    /// <summary>
    /// Gets the immutable globally unique identifier of the index, the identifier being
    /// assigned by the database when the index is created.
    /// </summary>
    Guid Id { get; }

    /// <summary>
    /// Gets a value indicating whether the index is invalidated, which means that the
    /// indexed element is no longer valid and should be reloaded from the database. The
    /// index can be invalidated by calling the <see cref="Invalidate()" /> method.
    /// </summary>
    bool IsInvalidated { get; }

    /// <summary>
    /// Invalidates the index, without forcing the indexed element to be reloaded from the
    /// database.
    /// </summary>
    void Invalidate();
}

Your third solution is not bad, but .NET Framework doesn't follow it. For example:

StringBuilder.Length
DateTime.Now

are properties, but don't expect them to remain constant every time.

What is consistently used across .NET Framework itself is this:

IEnumerable<T>.Count()

is a method, while:

IList<T>.Length

is a property. In the first case, doing stuff may require additional work, including, for example, querying the database. This additional work may take some time. In a case of a property, it is expected to take a short time: indeed, the length can change during the lifetime of the list, but it takes practically nothing to return it.


With classes, just looking at the code clearly shows that a value of a property won't change during the lifetime of the object. For instance,

public class Product
{
    private readonly int price;

    public Product(int price)
    {
        this.price = price;
    }

    public int Price
    {
        get
        {
            return this.price;
        }
    }
}

is clear: the price will remain the same.

Sadly, you can't apply the same pattern to an interface, and given that C# is not expressive enough in such case, documentation (including XML Documentation and UML diagrams) should be used to close the gap.

4

My 2 cents:

Option 3: As a non-C#-er, it wouldn't be much of a clue; However, judging by the quote you bring, it should be clear to proficient C# programmers that this means something. You should still add docs about this though.

Option 2: Unless you plan on adding events to every thing that may change, don't go there.

Other options:

  • Renaming the interface IXMutable, so it's clear that it can change its state. The only immutable value in your example is the id, which usually is assumed to be immutable even in mutable objects.
  • Not mentioning it. Interfaces are not as good at describing behaviour as we'd like them to be; Partly, that's because the language doesn't really let you describe things like "This method will always return the same value for a specific object," or "Calling this method with an argument x < 0 will throw an exception."
  • Except that there's a mechanism to expand the language and provide more precise information about constructs - Annotations/Attributes. They sometimes need some work, but the allow you to specify arbitrarily complex things about your methods. Find or invent an annotation that says "The value of this method/property can change throughout the lifetime of an object", and add it to the method. You might need to play some tricks to get the implementing methods to "inherit" it, and users may need to read the doc for that annotation, but it will be a tool that lets you say exactly what you want to say, instead of hinting at it.
aviv
  • 1,062
3

Another option would be to split the interface: assuming that after calling Invalidate() every invocation of IsInvalidated should return the same value true, there seems to be no reason to invoke IsInvalidated for the same part which invoked Invalidate().

So, I suggest, either you check for invalidation or you cause it, but it seems unreasonable to do both. Therefore it makes sense to offer one interface containing the Invalidate() operation to the first part and another interface for checking (IsInvalidated) to the other. Since it is pretty obvious from the signature of the first interface that it will cause the instance to become invalidated, the remaining question (for the second interface) is indeed quite general: how to specify whether the given type is immutable.

interface Invalidatable
{
    bool IsInvalidated { get; }
}

I know, this does not answer the question directly, but at least it reduces it to the question of how to mark some type immutable, which is quiet a common and understood problem. For example, by assuming that all types are mutable unless defined otherwise, you can conclude that the second interface (IsInvalidated) is mutable and therefore can change the value of IsInvalidated from time to time. On the other hand, suppose the first interface looked like this (pseudo-syntax):

[Immutable]
interface IX
{
    Guid Id { get; }
    void Invalidate();
}

Since it is marked immutable, you know that the Id will not change. Of course, calling invalidate will cause the state of the instance to change, but this change will not be observable through this interface.

proskor
  • 595
  • 1
  • 3
  • 13
2

I would prefer solution 3 over 1 and 2.

My problem with solution 1 is: what happens if there is no Invalidate method? Assume an interface with an IsValid property that returns DateTime.Now > MyExpirationDate; you might not need an explicit SetInvalid method here. What if a method affects multiple properties? Assume a connection type with IsOpen and IsConnected properties – both are affected by the Close method.

Solution 2: If the only point of the event is to inform developers that a similarly named property may return different values on each call, then I would strongly advice against it. You should keep your interfaces short and clear; moreover, not all implementation might be able to fire that event for you. I'll re-use my IsValid example from above: you would have to implement a Timer and fire an event when you reach MyExpirationDate. After all, if that event is part of your public interface, then users of the interface will expect that event to work.

That being said about these solutions: they aren't bad. The presence of either a method or an event WILL indicate that a similarly named property may return different values on each call. All I'm saying is that they alone are not enough to always convey that meaning.

Solution 3 is what I'd be going for. As aviv mentioned, this may only work for C# developers though. To me as a C#-dev, the fact that IsInvalidated is not a property immediately conveys the meaning of "that's not just a simple accessor, something is going on here". This does not apply to everyone though, and as MainMa pointed out, the .NET framework itself is not consistent here.

If you wanted to use solution 3, I would recommend to declare it a convention and have the whole team follow it. Documentation is essential too, I believe. In my opinion it's actually pretty simple to hint at changing values: "returns true if the value is still valid", "indicates whether the connection has already been opened" vs. "returns true if this object is valid". It wouldn't hurt at all though to be more explicit in the documentation.

So my advice would be:

Declare solution 3 a convention and consistenly follow it. Make it clear in the documentation of properties whether or not a property has changing values. Developers working with simple properties will correctly assume them to be non-changing (i.e. only change when object state is modified). Developers encountering a method that sounds like it could be a property (Count(), Length(), IsOpen()) know that someting's going on and (hopefully) read the method docs to understand what exactly the method does and how it behaves.

enzi
  • 228