views:

157

answers:

8

Is this an acceptable design ??

Abstract class

public abstract class SomethingBase
{ 
   public abstract int Method1(int number1);
   public abstract int Method2(int number2); 
} 

public class Class1 : SomethingBase
{
   public override int Method1(int number1)
   {
      //implementation
   }

   //i dont want this method in this class
   public override int Method2(int number2)
   {
        throw new NotImplementedException();
   }
}

public class Class2 : SomethingBase
{

   //i dont want this method in this class
   public override int Method1(int number1)
   {
     throw new NotImplementedException();
   }

   public override int Method2(int number2)
   {
    //implementation
    }
}

I mean the situation if I need method1 in my Class1 and the method2 not and vica verse for Class2. In fact the methods are excluding each other in the derived classes.

+4  A: 

I think there is something fundamentally wrong with your design. The two derived objects should only derive from a base class which genuinely represents commonality between the two. In your example, the base class does not represent commonality between Class1 and Class2.

AdamRalph
+8  A: 

It is not an acceptable design as it stands. To make it much more acceptable, you could implement two interfaces on the abstract class. One for Method1() and one for Method2(), and simply use the interface object to pass that around.

You really don't want to use the Class1 and Class2 definitions, as there is a danger of your clients calling the method that is not implemented. So I'd suggest to use interfaces and use these instead.

Wim Hollebrandse
read my mind word for word
Alon
+3  A: 

Is this an acceptable design ??

No. I can be acceptable in some situations (e.g. the ReadOnlyCollection in the FCL), but you should avoid it where possible.

You could use two interfaces: One with Method1 and one with Method2.

You should always follow the rule that derived classes should not harden the contract for the base class. They should be exchangable with the base class everywhere in your program.

winSharp93
+2  A: 

First of all, if you aren't defining any member variables in the abstract class I would probably just use interfaces.

Secondly I'd probably just have one method in the abstract class and each of the classes would implement it differently (with their own specific logic) as the signatures of Method1 and Method2 seem to be the same.

ruibm
+3  A: 

This is not an acceptable design as it is breaking the base class contract. Take for example the following client code

public void SomeFunction(SomethingBase x){
     x. Method2(1); // This will fail for Class1 but will pass for Class2. This is faulty.
}

Instead you can code to an interface and have one interface contain Method1 and the second interface contain Method2.

interface IM1{
     int Method1(int number1);
}

interface IM2{
     int Method2(int number2);
}

class Class1 : IM1{
     int Method1(int number1){
          // Implement your logic
     }
}

class Class2 : IM2{
     int Method2(int number2){
          // Implement your logic
     }
}

// Client Code
    public void SomeFunction(IM2 x){
         x. Method2(1); // This will work as this is the only thing this client wants.
    }
Hemanshu Bhojak
+3  A: 

I absolutely agree with the statement that the design you demonstrated should be avoided when possible in favor of interfaces.

But sometimes it is difficult to avoid or predict.

Classical example in .NET where approach you mentioned is used -- Stream class, where some children throws NotSupportedException for some abstract properties and methods. (like Position property in DeflateStream class.) But in order to implement that design properly you should have either virtual CanMethod1 properties or Method1Supported() methods. (As in TypeConverter -- usually when you have more complex cases, mainly when the method is supported only in some contexts.)

Regent
+4  A: 

It's breaks Liskov's Substitution Principle (a.k.a. LSP):

Let q(x) be a property provable about objects x of type T. Then q(y) should be true for objects y of type S where S is a subtype of T.

I also think of this as the IS-A rule. If a class B is derived from class A, it must support all the same functionality as class A, and not break any of its behavior in the process. If something consumes a class A object, you must be able to pass it a class B object instead, and not have it break.

For example, let's say your base class is-a Rectangle, and you derive a Square class from it. You've broken the LSP, because Rectangles can have a Width unequal to their Length, whereas a Square cannot.

In your case, however, your base class is abstract, so the LSP doesn't fit perfectly, but I think you understand what I'm getting at.

Mike Hanson
+1  A: 

Disagreed with majority, while upvoted Regent. Want to support and extend his position.

It is NOT ALWAYS a bad design to use NotSupportedException. You should weigh all the pros and cons.

What if you have a great variety of classes with nearly, but not exactly the same sets of methods? Streams are a good example. If you try to sort all that methods into interfaces, it 1) gives you an additional work of inventing intuitively clear and concise names for all of that interfaces, 2) forces the user of interfaces to understand what every one of them does.

Maybe throwing NotSupportedException in one or two methods would be clearer for a user of a class library then studying a complicated interface and class hierarchy.

Dmitry Tashkinov