29

Pretty straight-forward. I'm implementing an interface, but there's one property that is unnecessary for this class and, in fact, shouldn't be used. My initial idea was to just do something like:

int IFoo.Bar
{
    get { raise new NotImplementedException(); }
}

I suppose there's nothing wrong with this, per se, but it doesn't feel "right". Has anyone else come across a similar situation before? If so, how did you approach it?

  • 1
    I vaguely recall there being some semi-commonly used class in C# that implements an interface but explicitly states in the documentation that a certain method is not implemented. I'll try to see if I can find it. – Mage Xy Dec 29 '15 at 17:37
  • I'd definitely be interested to see that, if you can find it. – Chris Pratt Dec 29 '15 at 17:41
  • 23
    I can point out multiple cases of this in .NET's library - AND THEY'RE ALL RECOGNIZED AS BAD TERRIBLE MISTAKES. This is an iconic and common violation of the Liskov Substitution Principle - reasons not to violate LSP can be found in my answer here – Jimmy Hoffa Dec 29 '15 at 20:53
  • 3
    Are you required to implement this specific interface, or could you introduce a superinterface and use that? – Christopher Schultz Dec 29 '15 at 22:15
  • 6
    "one property that is unnecessary for this class" - Whether a part of an interface is necessary is up to the clients of the interface, not the implementers. If a class cannot reasonably implement a member of an interface, then the class is not the right fit for the interface. This may mean that the interface is poorly designed - probably trying to do too much - but that doesn't help the class. – Sebastian Redl Dec 31 '15 at 0:01
49

This is a classical example of how people decide to violate the Liskov Subtitution Principle. I strongly discourage it but would encourage possibly a different solution:

  1. Perhaps the class you're writing doesn't provide the functionality the interface prescribes if it doesn't have use of all the members of the interface.
  2. Alternatively, that interface may be doing multiple things and could be separated per the Interface Segregation Principle.

If the first is the case for you, just don't implement the interface on that class. Think of it like an electrical socket where the ground hole is unnecessary so it doesn't actually attach to ground. You don't plug anything with ground in and no big deal! But as soon as you use something which needs a ground - you could be in for a spectacular fail. Better off not punching a fake-ground hole in. So if your class doesn't actually do what the interface intends, don't implement the interface.


Here are a few quick bits from wikipedia:

Liskov Substitution Principle can be simply formulated as, "Don't strengthen pre-conditions, and don't weaken post-conditions".

More formally, the Liskov substitution principle (LSP) is a particular definition of a subtyping relation, called (strong) behavioral subtyping, that was initially introduced by Barbara Liskov in a 1987 conference keynote address entitled Data abstraction and hierarchy. It is a semantic rather than merely syntactic relation because it intends to guarantee semantic interoperability of types in a hierarchy, [...]

For semantic interoperability and substitutability between different implementations of the same contracts - you need them all to commit to the same behaviours.


Interface Segregation Principle speaks to the idea that interfaces should be separated into cohesive sets such that you don't require an interface that does many disparate things when you only want one facility. Think again of the interface of an electrical socket, it could have a thermostat also, but it would make it harder to install an electrical socket and may make it harder to use for non-heating purposes. Like an electrical socket with a thermostat, large interfaces are hard to implement and hard to use.

The interface-segregation principle (ISP) states that no client should be forced to depend on methods it does not use.[1] ISP splits interfaces which are very large into smaller and more specific ones so that clients will only have to know about the methods that are of interest to them.

  • Absolutely. This is probably why it didn't feel right to me, in the first place. Sometimes you just need a gentle reminder that you're doing something stupid. Thanks. – Chris Pratt Dec 30 '15 at 13:50
  • @ChrisPratt it's a really common mistake to make, that's why formalisms can be valuable - classifying code smells helps to more quickly identify them and recall solutions previously used. – Jimmy Hoffa Dec 30 '15 at 15:02
4

Looks fine to me, if this is your situation.

However, it seems to me that your interface (or use thereof) is broken if a deriving class doesn't actually implement all of it. Consider splitting that interface up.

Disclaimer: This requires multiple inheritance to do properly, and I have no idea whether C# supports that.

  • 6
    C# doesn't support multiple inheritance of classes, but it does support it for interfaces. – mgw854 Dec 29 '15 at 17:31
  • 2
    I think you're right. The interface is a contract after all, and even if I know this particular class won't be used in a way that it breaks anything that utilizes the interface if this property is disabled, that's not obvious. – Chris Pratt Dec 29 '15 at 17:39
  • @ChrisPratt: Yeah. – Lightness Races in Orbit Dec 29 '15 at 17:42
4

I have come across this situation. In fact as pointed out elsewhere the BCL has such instances... I'll try to provide better examples and provide some rationale:

When you have an already shipped interface that you keep for compatibility reasons and...

  • The interface contains members that are obsolete or discoraged. For instance BlockingCollection<T>.ICollection.SyncRoot (among others) while ICollection.SyncRoot is not obsolete per se, it will throw NotSupportedException.

  • The interface contains members that are documented to be optional, and that the implementation may throw the exception. For instance on MSDN regarding IEnumerator.Reset it says:

The Reset method is provided for COM interoperability. It does not necessarily need to be implemented; instead, the implementer can simply throw a NotSupportedException.

  • By a mistake of the design of the interface, it should have been more than one interface in the first place. It is a common pattern in BCL to implement Read Only versions of containers with NotSupportedException. I have done it myself, it is what is expected now... I make ICollection<T>.IsReadOnly return true so you can tell them appart. The correct design would have been to have a Readable version of the interface, and then the full interface inherits from that.

  • There is no better interface to use. For instance, I have a class that allows you access items by index, check if it contains an item and on what index, it has some size, you can copy it to an array... it seems a job for IList<T> but my class has a fixed size, and doens't support adding nor removing, so it works more like an array than a list. But there is no IArray<T> in the BCL.

  • The interface belongs to an API that is ported to multiple platforms, and in the implementation of a particular platform some parts of it are not supported. Ideally there would be some way to detect it, so that portable code that uses such API can decide whatever or not to call those parts... but if you call them, it is totally appropriate to get NotSupportedException. This is particularly true, if this is a port to a new platform that wasn't foreseen in the original design.


Also consider why is it not supported?

Sometimes InvalidOperationException is a better option. For instance one more way to add polymorphism in a class is by having various implementation of an internal interface and your code is choosing which one to instantiate depending on the parameters given in the constructor of the class. [This is particulary useful if you know that the set of options is fixed and you don't want to allow third party classes to be introduced by dependency injection.] I have done this to backport ThreadLocal because the tracking and non-tracking implementation are too far appart, and what does ThreadLocal.Values throw on the non-tracking implementation? InvalidOperationException even tho, it doesn't depend on the state of the object. In this case I introduced the class myself, and I knew that this method had to be implemented by just throwing an exception.

Sometimes a default value makes sense. For instance on the ICollection<T>.IsReadOnly mentioned above, it makes sense to just return ´true´ or ´false´ depending the case. So... what is the semantics of IFoo.Bar? there maybe some sensible default value to return.


Addendum: if you are in control of the interface (and you don't need to stay with it for compatibility) there shouldn't be a case where you have to throw NotSupportedException. Although, you may have to split the interface into two or more smaller interfaces to have the right fit for your case, which may lead to "pollution" in the extreme situations.

0

Has anyone else come across a similar situation before?

Yes, the .Net library designers did. The Dictionary class does what you're doing. It uses explicit implementation to effectively hide* some of the IDictionary methods. It's explained better here, but to summarize, in order to use the Dictionary's Add, CopyTo, or Remove methods that take KeyValuePairs, you first have to cast the object to an IDictionary.

* It's not "hiding" the methods in the strict sense of the word "hide" as Microsoft uses it. But I don't know a better term in this instance.

  • ... which is terrible. – Lightness Races in Orbit Dec 29 '15 at 21:05
  • Why the downvote? – user2023861 Dec 29 '15 at 21:11
  • 2
    Probably because you didn't answer the question. Ish. Sort of. – Lightness Races in Orbit Dec 29 '15 at 21:16
  • @LightnessRacesinOrbit, I updated my answer to make it more clear. I hope it helps the OP. – user2023861 Dec 29 '15 at 21:18
  • 2
    Seems like it answers the question to me. The fact that it might be a good or bad idea doesn't seem to be within the scope of the question, but might still be useful for answers to indicate. – Ellesedil Dec 29 '15 at 23:30
-6

You could always just implement and return a benign value or default value. After all it's just a property. It could return 0 (the default property of int), or whatever value makes sense in your implementation (int.MinValue, int.MaxValue, 1, 42, etc...)

//We don't officially implement this property
int IFoo.Bar
{
     { get; }
}

Throwing an exception seems like bad form.

  • 2
    Why? How is returning incorrect data any better? – Lightness Races in Orbit Dec 29 '15 at 23:19
  • 1
    This breaks LSP: see Jimmy Hoffa's answer for an explanation why. – user22815 Dec 29 '15 at 23:23
  • 5
    If there are no possible correct return values, it's better to throw an exception than to return an incorrect value. No matter what you do, your program is going to malfunction if it accidentally invokes this property. But if you throw an exception, then it will be obvious why it's malfunctioning. – Tanner Swett Dec 30 '15 at 0:39
  • I don't now how the value would be incorrect when your the one implementing the interface! It's your implementation, it can be whatever you want. – Jon Raynor Dec 30 '15 at 14:46
  • What if the correct value was unknown upon compile time? – Andy Dec 31 '15 at 7:49

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.