20

I have an interface (move) that should move some shapes.

interface Move { move(); }
abstract class Shape : Move

class Circle : Shape
class Square : Shape
class Triangle : Shape

My doubt is, I must have an interface which moves Shapes but only Circle and Triangle should be able to be moved, so how do I "remove" the interface from Square? Should I remove the interface from Shape and add it manually on Circle and Triangle? I'm kinda confused with this. Hope someone can help me out.

Mike Perrenoud
  • 66,820
  • 29
  • 157
  • 232
sok
  • 612
  • 2
  • 10
  • 24

6 Answers6

34

You should setup your classes like this:

interface IMovable { move(); } 
abstract class Shape : { } 

class Circle : Shape, IMovable { } 
class Square : Shape { } 
class Triangle : Shape, IMovable { } 

If not every shape can be moved then Shape must not implement the interface. Also note I renamed your interface to IMovable, it's not a huge deal but it's more accepted and a better naming convention.

oɔɯǝɹ
  • 7,219
  • 7
  • 58
  • 69
Mike Perrenoud
  • 66,820
  • 29
  • 157
  • 232
  • 4
    This. The answer to the question "how do I un-inherit some base behavior" is always "don't inherit from it in the first place". – KeithS Oct 01 '12 at 14:46
  • @KeithS, that is correct. Stopping inheritance is just the inverse way to look at inheritance. So, hopefully this will help the OP think the other way moving forward. :) – Mike Perrenoud Oct 01 '12 at 14:47
  • @smartcaveman, are you saying renaming the interface to `IMovable`? – Mike Perrenoud Oct 01 '12 at 16:35
  • 1
    @Mike, yeah. Because it's not a move, it's an object that **can** be moved. It seems to follow convention more: e.g. `IEnumerable`, `IQueryable`, `IObservable`, `IEquatable`, `IComparable`, `IFormattable`, etc. – smartcaveman Oct 01 '12 at 16:51
27

You can't remove an interface from an inheritance tree.

What you model appears to need two abstract classes - Shape and MovableShape.

interface IMove { move(); } 
abstract class Shape : {} 
abstract class MovableShape : IMove, Shape {} 

class Circle : MovableShape{}
class Square : Shape{}
class Triangle : MovableShape{}
Oded
  • 489,969
  • 99
  • 883
  • 1,009
  • 3
    @NadirSampaoli, you're __opinion__ is duly noted, but be careful as this would turn into a very subjective conversation. One thing to note is that __I +1'd this answer as well because it's quite correct__. – Mike Perrenoud Oct 01 '12 at 14:25
  • 3
    @Mike I apologize, in fact my comment sounds bold; I should have appended an "in my opinion". – Nadir Sampaoli Oct 01 '12 at 14:35
  • 2
    +1 @Mike - Both answers are _correct_. Which one is more suitable for the OP? There isn't enough information in the question for an answer to be determined. – Oded Oct 01 '12 at 14:35
  • 1
    @Oded, thank you, I'm glad we can agree. I think often times on this forum we forget (for many reasons I'm sure) that there are many correct ways of doing the same thing and there is often not enough information to be __sure__ which is the appropriate one. However, that's why we can +1 each others answers and let the community know, hey their answer is right too! I feel like the competition for reputation has become more important lately than being a community. You? – Mike Perrenoud Oct 01 '12 at 14:39
  • 2
    @Mike - When it comes to reputation, I can't really comment too much, only to say that on the day to day its become irrelevant to me. Community is important and trying to foster it instead of seeing the site as a "competition" is right. (BTW - there is the [Sportsmanship badge](http://stackoverflow.com/badges/805/sportsmanship) to encourage community thinking). – Oded Oct 01 '12 at 14:42
  • 1
    @Oded, thanks! That's how I'm starting to feel. I look forward to continuing to work with you on here in the future! – Mike Perrenoud Oct 01 '12 at 14:43
  • 2
    @NadirSampaoli With regards to the discussion of Mike and Oded, I think Nadir deserves some credit for giving a reasonable answer to Mikes reply, instead of bursting out in flames (which happens all to often I guess). – Timo Oct 01 '12 at 17:52
  • You'll see some disadvantages when you introduce additional behavior for a Shape. Let's say you introduce also "Turn". With this approach, you suddenly need 4 abstract classes (Shape, MovableShape, TurnableShape, MovableAndTurnableShape). If you introduce 2 more options, you already need 16 abstract classes. One of the advantages of this approach is that you can implement already code for all subclasses. If a big part of the "Move" code is always the same for all classes, you can implement it in the abstract class and only expose an abstratc method for the remaining logic to be implemented – Matt Oct 23 '13 at 14:50
3

You should make yourself more familiar with the ideas behind interfaces, classes and OO in general. What you are trying to tell is the following:

  • Every shape can be moved.
  • A Square is a shape.
  • But a Square cannot be moved.

Obivously that makes not sense. So you have to adjust your class design. Either every shape can be moved, that Shape (and Square) should implement Move, or not every shape can be moved, then Shape should not implement Move.

Achim
  • 15,415
  • 15
  • 80
  • 144
3

Try this out:

interface IMove { move(); }
abstract class Shape { }

class Circle : Shape, IMove { }
class Square : Shape { }
class Triangle : Shape, IMove { }
Furqan Safdar
  • 16,260
  • 13
  • 59
  • 93
2

Other option could be just implement IMove.Move method in the Shape class and throw a NotSupportedException by default.

public abstract class Shape : IMove 
{
     public virtual void Move()
     { 
         throw new NotSupportedException();
     }
}

So at the end of the day, "any shape could be movable" but "a movable shape should provide its own implementation of how to move it".

Finally, let's imagine there're a bunch of shapes that are moved in the same way. You'd create a DefaultMovableShape abstract class deriving Shape, which overrides Shape.Move virtual method.

public abstract class DefaultMovableShape 
{
     public override void Move()
     {
           // Do stuff
     }
}
Matías Fidemraizer
  • 63,804
  • 18
  • 124
  • 206
  • While this works, I'd advise against it unless your spec for the `Move` function explicitly state at the interface layer that `Move` will may throw an exception in the case it isn't applicable for the underlying object. Of course, at that point you should question why it implements the interface at all and you likely have a design issue (see accepted answer). – Anthony Oct 01 '12 at 13:53
  • @Anthony I'm agreed. I just wanted to give an option. I believe that there're reasons to think that accepted answer is the right way of doing that, but also mine, in some situation, may done the job. It depends on how you think the problem: "a shape isn't movable" or "a shape may be movable". – Matías Fidemraizer Oct 01 '12 at 14:20
  • @Anthony I've double-checked your comment. This statement should be in `Shape` class documentation nor `IMove` one. Shape "may move" because a Shape can provide an implementation of "how to move". So if you're trying to move a Shape that can't be moved, then this Shape "doesn't support moving". – Matías Fidemraizer Oct 01 '12 at 14:23
2

The best answer will depend on what the use case and environment for these classes will be. As part of a team developing an app or framework, adopting the design patterns used by that team is preferable to seeking a 'perfect' solution, since it will make it easier for others to adopt and maintain your code.

How you expect these classes to be used and extended is also important. Would you expect 'Square' to need to be movable in the future? Is the movability of a Shape always static, or might it be more useful as a dynamic attribute? Does Move() have any value for classes that are not Shapes? If movability may be useful as a dynamic attribute, consider this:

public abstract class Shape
{
     public bool isMovable()
     {
         return false;
     }

     public virtual void Move()
     { 
         if (!isMovable() {
             throw new NotSupportedException();
         } else {
             throw new BadSubclassException();
         }
     }
}

Your subclasses can then override isMovable to provide either static or dynamic behavior, and can be further modified or subclassed over time, so long as your documentation makes clear that isMoveable should always precede a call to Move. The default behavior should be based on the expectations of others you expect to use your code, based on how they've implemented related design patterns.

A good example of the challenge of making these decisions can be found by looking at the history of how mutability of collection classes has evolved in different frameworks. There have been designs where the mutable classes (sets, arrays, dictionaries, etc.)have been the base class, with immutability implemented in subclasses, as well as the reverse. There are valid arguments for both approaches, as well as a dynamic approach, but the most important factor for the user of a framework is consistency, because what is correct is really a matter of what is easiest to use, providing safety and performance are not compromised.

Steven McGrath
  • 1,717
  • 11
  • 20