Proposal: Allow getters and setters to have different signature in interfaces

[pongba]

The lack of this distinction has caused so much pain and confusion, see: here, here and here.

I wanted to be able to specify IReadOnlyMap and IMap, and naturally expected this to work:

interface IReadOnlyMap
{
    double this[string key] { get; }
}

interface IMap : IReadOnlyMap
{
    double this[string key] { set; }
}

I thought this would work because getter and setter are distinct methods with distinct names, but instead I was surprised to find out that there is name hiding.

People typically suggest to use new, i.e.

interface IMap : IReadOnlyMap
{
    new double this[string key] { get; set; }
}

This is semantically broken, as noted by one SO user:

the new modifier is fundamentally wrong since it says that we have an unrelated property. It does not "extend" the existing property.

It's fundamentally wrong to use new here for both aesthetic and practical (since it allow separate explicit implementation of the getter) reasons. Bad, wrong, period.

I spent 1 hour searching and failed to find a semantically correct approach within current language design, hence the proposal here.

[theunrepentantgeek]

I'd suggest the problem is having IMap extend IReadOnlyMap.

When I read your sample, IReadOnlyMap means "this is a map that will never be changed" - so my consuming code could rely on it never changing (and, for example, cache values if required).

By contrast, IMap means (to me) "a map that might change", so my code needs to allow for it being changed (and would, to reuse the example above, not consider caching values).

In a code review I would flag code where IMap descend from IReadOnlyMap as confusing because because then we'd have an implementation of IReadOnlyMap that wasn't actually read only, but was expected to change.

[yaakov-h]

@theunrepentantgeek ReadOnly and Immutable are not always the same thing. There's precedent in .NET already for readonlies being mutable, e.g. List<T> implementing IReadOnlyList<T>.

Edit: Oh, and ReadOnlySpan<T>.

[khm1600]

I thought this would work because getter and setter are distinct methods with distinct names

But properties (which are what indexers compile to) are not.

Currently, this

interface IReadOnlyMap
{
    double this[string key] { get; }
}
interface IMap : IReadOnlyMap
{
    double this[string key] { set; }
}

means

interface IReadOnlyMap
{
    double get_Item(string);
    property double Item(string) with get : get_Item
}
interface IMap : IReadOnlyMap
{
    void set_Item(string, double);
    property double Item(string) with set : set_Item
}

and what you are proposing would make it

interface IReadOnlyMap
{
    double get_Item(string);
    property double Item(string) with get : get_Item
}
interface IMap : IReadOnlyMap
{
    void set_Item(string, double);
    property double Item(string) with
        get : IReadOnlyMap.get_Item
        set : set_Item
}

which I believe is impossible without clr changes.

[Austin-bryan]

So is this adding a set or replacing the set with a get? It's not clear from the source code which can make things more confusing if you think the method is set only.

Also, I think this violates LSP because you can pass IMap into a method expecting an IReadonlyMap and the method would be expecting it to be readonly but here it wouldn't be the case, and that could be bad.

[yaakov-h]

1. Adding - you can never remove or replace, that's impossible.
2. See my comment above about mutability. The method would expect it to be read-only and it would be read-only, but it wouldn't be immutable.

[DavidArno]

Post retracted as the code doesn't work as @svick points out below.

[DavidArno]

@yaakov-h,

There's precedent in .NET already for readonlies being mutable...

It's a bad precedent. Therefore it's a precedent that we should actively ignore.

[svick]

@DavidArno

If one favours composition over inheritance, this problem goes away

1. I don't think that's favoring composition over inheritance, since you're not using composition at all (a class that delegates its functionality to objects it contains).
2. Write-only properties (and indexers) are nigh useless, which is why they are very rare in practice. For example, with your code, there is no way to increase a value that's in the map though an interface (and it doesn't work even with where T : IReadableMap, IMutableMap).

It's a bad precedent. Therefore it's a precedent that we should actively ignore.

Care to explain why? Making everything immutable is not practical and having read-only view over potentially mutable data is very useful, especially if you want to separate the concerns in your code.

[DavidArno]

@svick,

Write-only properties (and indexers) are nigh useless, which is why they are very rare in practice. For example, with your code, there is no way to increase a value...

Good point. The code compiles just fine but is then unusable in many situations due to the compiler not being able to resolve whether the getter or setter is referred to in eg map["1"] = map["1" + 1.

[DavidArno]

@svick,

Care to explain why?

It's all to do with the "pit of success" and thus making "readonly" the default state and the fact that many folk (incorrectly, but frequently) treat read-only as immutable.

So a better design would have been IList<T> (read-only version) and IMutableList<T>. We can't fix that in the BCL, but what the BCL does shouldn't be a precedent for what we do in our own code.

[Austin-bryan]

@DavidArno

but what the BCL does shouldn't be a precedent for what we do in our own code.

I highly agree. Pretty sure it's a logical fallacy to assume that because someone else, or a lot of people, or a lot of smart people do it, then it's the best thing to do. Everyone is subject to mistakes, even the writers of .NET

[jnm2]

Whether or not you use the name Readable or ReadOnly in your own code, you do know what to expect in .NET when you see ReadOnly around the ecosystem. It always means Readable, nothing more. Immutable is the naming convention that tells you that the thing will not change.

[Neme12]

So a better design would have been IList (read-only version) and IMutableList. We can't fix that in the BCL, but what the BCL does shouldn't be a precedent for what we do in our own code.

That would be my preference too but is that all this is about? Are we seriously just arguing about the naming here?

[theunrepentantgeek]

This is kind of my fault. 😞

I was trying to make the point that the interface inheritance used in the original example didn't make any sense because the two interfaces had very different semantics that shouldn't be conflated together into an inheritance structure.

But, I didn't explain that well, everyone has gotten distracted by ReadOnly vs Immutable, and my original point seems to have been lost.

[pongba]

@theunrepentantgeek well I think it's a very good discussion, since I never realized that there's this distinction, and it's a very important distinction.

[Neme12]

But as explained in point 1, there is no similar requirement for a mutable interface to mandate a read-only state too; there is no requirement for interface inheritance in this case.

As stated before, IReadOnly or IReadable does not mean that the object is immutable and will never change. It just means you have a read-only access to it - you cannot change it.

Imagine writing a function that takes a list and only needs to read some data from it, once. Should it take a IList or IWritableList or IReadWriteList? Absolutely not. What if the caller only has an immutable one? They should be able to pass it in. After all, the list is only used for reading. The answer is that the function should take an IReadOnlyList, because it needs a list for reading. It does not care at all whether the caller has a mutable list that they will then change later, or whether it's immutable. All it cares about is that it can read the list.

Under your proposal, if the caller has a mutable list interface, let's say IList, they cannot pass it into this function. I absolutely do not understand what you're arguing for or why you think the caller shouldn't be able to pass a mutable list for reading. A read-write list should always inherit from a read-only list interface, whether you call it IReadOnly, IReadable or whatever.

[DavidArno]

@Neme12,

Sorry, but I've absolutely no idea what you are trying to argue. It is irrelevant whether we have:

interface IReadOnlyList {}
interface IList : IReadOnlyList {}
class List : IList {}

or

interface IReadOnlyList {}
interface IList {}
class List : IList, IReadOnlyList {}

in both cases, I can supply List to a method that wants IReadOnlyList and it will be able to read from it. So I do not understand how you arrive at "A read-write list should always inherit from a read-only list interface". This is not a statement that can be objectively supported in any way as far as I can see. It appears to be pure dogma.

Could you try to explain your argument differently, please? I might get then.

[YairHalberstadt]

@DavidArno
Can you give an example of any class which you would want to implement IList, but not IReadOnlyList?

[khm1600]

@DavidArno

In general terms, there doesn't have to be a progression from read-only to read/write...

I was assuming that r/w extending r/o is possible. (e.g. r/o is created no later than r/w, such change is acceptable, etc.)

But as explained in point 1, there is no similar requirement for a mutable interface to mandate a read-only state too; there is no requirement for interface inheritance in this case.

I don't think it is too much for something requiring read-write to require readable. On the other hand, something read-write not requiring readable is weird.

due to the way the language and CLR work, trying to do such an inheritance model creates the very issue that the OP was addressing

Not extending the interface does not solve the problem either. Users still can implement getters differently. And it actually creates more problems: I can now implement the r/w interface and not implement the r/o one.

There isn't an inheritance chain there of...

Actually there is. (except that IReadOnlyList<T> is not in the chain for backwards compatibility reasons)

instead, List implements each directly:

Maybe the writer of this just want to be specific about what interfaces are implemented? But it looks like some random interfaces that List<T> implemented to me: it's not a exhaustive list, nor is it a minimal one, it does not reflect the specs, nor does it reflect the code. If it comes from the docs, then the fact that the F# declaration is different further supports this idea (that it's just a random list).

[Neme12]

@DavidArno In the first case, I can pass an IList to a function that takes an IReadOnlyList. In the second case, I cannot.

[GeirGrusom]

@Neme12 you can pass List to a function that expects IReadOnlyList.

[Neme12]

I said

IList

(notice the I)

This is important if you're dealing with interfaces, either that you got from someone else or you just prefer to always work with interfaces, like it's conventional to do in Java.

[DavidArno]

@Neme12,

I was going to ask for a use-case where you'd do that. But of course, IList<T> doesn't inherit from IReadOnlyList<T>, so that would be an unfair thing to ask. 😆

[DavidArno]

@YairHalberstadt

Can you give an example of any class which you would want to implement IList, but not IReadOnlyList?

I'm sure I could dream up some (likely daft) example. But I don't really get the point. It's useful to have a list implement both a readable view and a mutable view of itself. Just don't use interface inheritance to do so as it's not necessary and can cause all sorts of problems as per the OP's example.

[YairHalberstadt]

On the other hand @DavidArno, a lot of read only lists implement IList anyway. The reason being that a lot of functions that could accept an IReadOnlyList only accept an IList. The reason for that is that it would be a breaking change to accept an IReadOnlyList in place of an IList, since IList doesn't implement IReadOnlyList, despite the fact that (as you yourself admit) pretty much every class that implements IList ought to implement IReadOnlyList.

[YairHalberstadt]

Similarly, if you have a function that accepts an IList, you can't call a function that accepts an IReadOnlyList without some sort of hack.

This is despite the fact that 99% of all IList implementations do (or at least should) implement IReadOnlyList.

[DavidArno]

@YairHalberstadt,

On the other hand @DavidArno, a lot of read only lists implement IList anyway. The reason being that a lot of functions that could accept an IReadOnlyList only accept an IList…

I know, and I find that really sad. I understand why eg ImmutableList>T> implements IList<T>, it's because many devs are lazy and just use IList<T> rather than thinking hard about the type they use and only using what they need, eg IEnumerable<T>, IReadOnlyList<T> etc.

I would have preferred that the authors of those immutable types had stuck to their principles and had said "if you want to use this type, you'll have to fix your code". But so much of the .NET ecosystem is about pandering to those that don't want to change things, so they'd have limited their market if they had done.

Similarly, if you have a function that accepts an IList, you can't call a function that accepts an IReadOnlyList without some sort of hack.

If/when we get shapes, this may be addressed. Rather than using inheritance, I can then create a shape composed of multiple interfaces. Then my method can require an SReadableAndMutableList, which fits the IList I need and meets the needs of a function I call that wants an IReadOnlyList. One day, maybe...

[aka-nse]

Even if every developer use the required minimum interfaces, logical mismatch between IList<T> and IReadOnlyList<T> will cause misfortune yet. Once the instance is received by IList<T> variable, it can no longer be passed to anyone as IReadOnlyList<T> instance.

"[readable and writable list] is [readable list]" is always correct logically, but code can never understand that even if you claim in the document.

[Thaina]

@glassgrass The point is I think code should understand that. And we could make it understand if we make IList also be IReadOnlyList by interface inheritance. But we can't because the design flaw from the start that we should just admitted that we were wrong. C# design team has made mistake since C# 1 so it should not take it as a reason to argue that because it like this so it right

[YairHalberstadt]

With C# 8.0 you can do this:

interface IReadOnlyMap
{
    double this[string key] { get; }
}

interface IMap : IReadOnlyMap
{
    double IReadOnlyMap.this[string key] => this[key];
    new double this[string key] { get; set; }
}

This has two advantages:

1. It makes it clear how the two indexers are related.
2. If you implicitly implement IMap.this[string], you don't need to implement IReadOnlyMap.this[string] as well.