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,138
  • 18
  • 29
  • Doesn't `IsInvalidated` need a `private set`? – James Aug 24 '13 at 12:41
  • 2
    @James: Not necessarily, neither in the interface declaration, nor in an implementation: `class Foo : IFoo { private bool isInvalidated; public bool IsInvalidated { get { return isInvalidated; } } public void Invalidate() { isInvalidated = true; } }` – stakx Aug 24 '13 at 12:43
  • @James Properties in interfaces are not the same as automatic properties, even though they use the same syntax. – svick Aug 24 '13 at 14:57
  • I was wondering: do interfaces "have the power" to impose such behavior? Shouldn't this behaviour be delegated to each implementation? I think that if you want to enforce things to that level, you should consider to create an abstract class so that subtypes would inherit from it, insead of implementing a given interface. (by the way, does my rationale make sense?) – heltonbiker Aug 30 '13 at 14:59
  • @heltonbiker Unfortunately, most languages (I know) do not allow to express such constraints on types, but they definitely **should**. This is a matter of specification, not implementation. In my opition, what's missing is the possibility to express *class invariants* and *post-condition* directly in the language. Ideally, we should never have to worry about `InvalidStateException`s, for example, but I'm not sure whether this is even theoretically possible. Would be nice, though. – proskor Aug 30 '13 at 16:08
  • @proskor Well, most languages maybe, but doesn't C# (the language mentioned in the question) allows for that? There are quite a handful of specifiers like `virtual`, `final`, `protected`, `readonly` and so on... – heltonbiker Aug 30 '13 at 19:02
  • @heltonbiker Sure, but that's not enough! For example, how would you specify that some method, say `foo()`, should return an even integer number? There is no syntax in C# to define such a constraint. Of course, you can define a custom type like `EvenInteger`, but this approach has its limits. What if you'd like to specify that the method should return an even integer between 0 and 100 or a prime number otherwise? Or how do you specify that the first parameter must be divisible by the second one? And so on... – proskor Aug 30 '13 at 20:49
  • @proskor I don't think the OP asked for such thing... But I understand the general rationale, and sure it might matter depending on context... And for your answers, at least in C# you might put some code in the getters of properties. – heltonbiker Aug 30 '13 at 21:08
  • @heltonbiker That's true, I can put validation code into getters and setters, but it doesn't help me specifying the semantics of the interface. In C#, you can only say that there is a getter and/or setter for this in the interface, and it is of a some type, and that's it. – proskor Aug 30 '13 at 21:17
  • How about renaming your property `IsCurrentlyValid`? That would pretty well suggest that the value of the property could change. – supercat Feb 25 '14 at 00:00

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.

Arseni Mourzenko
  • 134,780
  • 31
  • 343
  • 513
  • Even the “no additional work” guideline is not used absolutely consistently. For example, `Lazy.Value` may take a very long time to compute (when it's accessed for the first time). – svick Aug 24 '13 at 15:02
  • 1
    In my opinion, it doesn't matter if the .NET framework follows the convention of solution 3. I expect developers to be smart enough to know if they're working with in-house types or framework types. Devs that don't know that don't read docs and thus solution 4 wouldn't help either. If solution 3 is implemented as a company / team convention, then I believe it doesn't matter whether or not other APIs follow it too (although it would be very much appreciated, of course). – enzi Aug 30 '13 at 11:15
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
  • 8
  • 9
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
  • 585
  • 1
  • 3
  • 12
  • Not a bad idea! However, I would argue that it is wrong to mark an interface as `[Immutable]` as long as it encompasses methods whose sole purpose is to cause a mutation. I would move the `Invalidate()` method to the `Invalidatable` interface. – stakx Aug 30 '13 at 17:50
  • 1
    @stakx I disagree. :) I think you confuse the concepts of *immutability* and *purity* (absence of side effects). In fact, from the viewpoint of the proposed interface IX, there is no observable mutation at all. Whenever you call `Id`, you'll get the same value every time. The same applies to `Invalidate()` (you get nothing, since its return type is `void`). Therefore, the type is immutable. Of course, you'll have *side effects*, but you will not be able to *observe* them through the interface `IX`, so they will not have any impact on the clients of `IX`. – proskor Aug 30 '13 at 21:07
  • OK, we can agree that `IX` is immutable. And that they are side effects; they are just distributed between two split interfaces such that clients don't need to care (in the case of `IX`) or that it's obvious what the side effect might be (in the case of `Invalidatable`). But why not move `Invalidate` over to `Invalidatable`? That would make `IX` immutable *and* pure, and `Invalidatable` would not become more mutable, nor more impure (because it is both of these already). – stakx Aug 31 '13 at 07:48
  • (I understand that in a real-world scenario, the nature of the interface split would probably depend more on practical use cases than on technical matters, but I'm trying to fully understand your reasoning here.) – stakx Aug 31 '13 at 07:49
  • 1
    @stakx First of all, you asked about further possibilities to somehow make the semantics of your interface more explicit. My idea was to reduce the problem to that of immutability, which required splitting the interface. But not only was the split required to make one interface immutable, it would also make great sense, because **there is no reason to invoke both `Invalidate()` and `IsInvalidated` by the same consumer of the interface**. If you call `Invalidate()`, you should know it becomes invalidated, so why check it? Thus you can provide two distinct interfaces for different purposes. – proskor Aug 31 '13 at 09:42
  • OK, I can see now where you're coming from. Your answer provides a valuable idea. While splitting up an interface might not work in every case (I'll keep it in mind and see whether it works as a general solution to the problem) and might not suit everyone's taste (e.g. I've had bad experiences with a large framework that split up functionality into many interfaces in a very unnatural, ad hoc way), it certainly sounds like a reasonable option. Thanks for that! – stakx Aug 31 '13 at 10:02
  • I'm glad I could help. :) However, I would argue that the concept of "grouping similar things together" is a little bit misused here. The subtle problem is that the notion of *similarity* in this case is not technical at all. To be precise, I don't think that *causing* invalidation and *checking* for invalidation have something to do with each other. The fact that they both have something to do with invalidation does not really make them *similar* from a "technical viewpoint". – proskor Aug 31 '13 at 10:04
  • Perhaps I need to explain what I meant by "technical viewpoint"; namely, that the actual consumers' usage patterns are not considered, and one merely decides (based on their similar names, and on purity/immutability distribution) that `Invalidate` and `IsInvalidated` should go together. Arguing from such a standpoint has helped me understand in what ways your answer goes beyond it. – stakx Aug 31 '13 at 10:12
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
  • 2
  • 6
  • This question has received very good answers, and it's a pity I can accept only one of them. I have chosen your answer because it provides a good summary of some points raised in the other answers, and because it's more or less what I have decided to do. Thanks to everyone who has posted an answer! – stakx Feb 14 '15 at 22:12