5

I have a question about Derivation , polymorphism and method signature

I have class

public abstract class PaymentMethod
{   
    public abstract float GetSalary (Employee employee,Vehicule vehicule)
}

and two others

public class MarinePaymentMethod : PayementMethod
{
    public override float GetSalary (Sailor sailor,Boat boat)
}

public class AirPaymentMethod : PayementMethod
{
    public override float GetSalary (Pilot pilot,Plane plane)
}

We assume also that :

public class Sailor : Employee{}
public class Pilot : Employee{}
public class Boat: Vehicule{}
public class Plane: Vehicule{}

So, the "problem" is that this code does not compile , because the signatures are not the same.

I am force to keep the base signature GetSalary(Employee employee, Vehicule vehicule)

and then I must cast in the Derived Payment Method, so that I can use the specific members of Pilot, Sailor, Boat, Plane in these specific payment method.

My first question is: Isn't it a bad smell to cast continuously?

My second question is: How to make a more elegant code Design ? I was thinking about Generics, and create a class like this:

public abstract class PaymentMethod<T, U> where T: Employee where  U: Vehicule 

But then in my code I realize I must put generic almost everywhere I use a payment mehod class and it make the code heavy. Any other solutions ?

Thanks a lot

Christos
  • 53,228
  • 8
  • 76
  • 108
Levis Marc
  • 51
  • 2
  • 2
    Don't use float for currency. Currency is decimal so use a decimal type to represent it not binary floating point. – Ben Robinson Dec 16 '14 at 10:51
  • Why are you forced the keep the base signature? It looks to me that you really shouldn't be trying to override the method in the derived classes. If the getsalary methods in the derived classes need to utilise some base GetSalary functionality just expose the method on the base as protected rather than making it abstract. – DoctorMick Dec 16 '14 at 10:52
  • Yes, it's a bad smell - basically each of your `PaymentMethod` implementations can only be used in *some* cases where one would expect `PaymentMethod` to be used. Yes, your alternative approach sounds reasonable. It's not clear what you mean by "it makes the code heavy". – Jon Skeet Dec 16 '14 at 10:53
  • 1
    Well, ask yourself if `GetSalary(new Sailor(), new Plane())` makes sense. If not then, probably, there is something wrong with design (it doesn't matter if you use generics or not). A **protected base class method** and several **public specializations** in derived classes...maybe (example [here](http://stackoverflow.com/a/23911451/1207195)). Do you ever need to call base unspecialized method? Do you ever need to call derived specialized methods? Finally don't forget **you're not overriding base class methods but overloading them**. Keep it (if you need such methods. – Adriano Repetti Dec 16 '14 at 10:56
  • To Doctor Mick: I need to call the method in an algorythm which is common to all derived classes. If I use real type in the signature, I must duplicate the algoythm. To Jon: I 'll have to write PaymentMethod in my code if I use it as a methode parameter. I made an example with 2 parameters, but often I have 3-4 paramteters like this. Really it's ugly. – Levis Marc Dec 16 '14 at 13:03
  • 1
    Can you post a snippet of the calling code? I'm pretty sure there is a much neater solution to this, probably moving the GetSalary method on to the Sailor/Pilot classes. If you've introduced the PaymentMethod class simply to encapsulate the common algorythm then that can be tackled with a collaborator. Hard to say for certain without knowing how you're calling it tho. – DoctorMick Dec 16 '14 at 13:30

1 Answers1

1

Personally, I'd do it this way:

public abstract  class PaymentMethod { 
    public decimal GetSalary(Employee employee, Vehicle vehicle) {
        return GetSalaryImpl(employee, vehicle);
    }    
    protected abstract decimal GetSalaryImpl(Employee employee, Vehicle vehicle);
}
public class MarinePaymentMethod : PaymentMethod {
    public decimal GetSalary(Sailor sailor,Boat boat)
    { throw new NotImplementedException(); /* your code here */ }
    protected override decimal GetSalaryImpl(Employee employee, Vehicle vehicle) {
        return GetSalary((Sailor)employee, (Boat)vehicle);
    }
}    
public class AirPaymentMethod : PaymentMethod {
    public decimal GetSalary(Pilot pilot, Plane plane)
    { throw new NotImplementedException(); /* your code here */ }
    protected override decimal GetSalaryImpl(Employee employee, Vehicle vehicle) {
        return GetSalary((Pilot)employee, (Plane)vehicle);
    }
}
public class Employee {}
public class Vehicle{}
public class Sailor : Employee{}
public class Pilot : Employee{}
public class Boat: Vehicle{}
public class Plane: Vehicle{}

This then works for both polymorphism and overload approaches - although it is unusual that a method that takes an Employee requires a specific type of employee.

Marc Gravell
  • 1,026,079
  • 266
  • 2,566
  • 2,900
  • In my code , I have an "algorythm" which uses bases classes. In this code, I would like to use template methods, that are overriden in derived classes , and derived arguments, because I need the derived implementation in the overriden methods. It seems your proposal does the job. It's an optimized cast strategy, as the cast is made only once ! – Levis Marc Dec 16 '14 at 12:41
  • "it is unusual that a method that takes an Employee requires a specific type of employee." The method takes an employee because it is an abstract method , which is used in a generic algorythm common to all employees. However , the overriden method does a job which requires a specific employee. That's my problem ! – Levis Marc Dec 16 '14 at 12:49