views:

45

answers:

1

How do you think about data access code like this:

public void AddCusotmer(Cusotmer customer)
{
   //save customer into database
   ...

   // save payment type
   SavePaymentType(customer);

   //save other data
   ...
}

private void SavePaymentType(Customer customer)
{
   if(customer.PaymentType is XXXPayment)
   {
      var payment = customer.PaymentType as XXXPayment;
      //save payment to XXXPayments table in db
      ...
   }
   else if(customer.PaymentType is YYYPayment)
   {
      var payment = customer.PaymentType as XXXPayment;
      //save payment to YYYPayments table in db
      ...
   }
   ...
}

Personally, I'm not feeling very well with codes like this (using "is" to detect type to decide what to do), but Robert Martin the author says it's OK for it's only in DAL, so a little bit violation of OCP is acceptable.

How do you think?

A: 

The code like this doesn't smell good for me.
You are probably doing your own O/R-M so don't know all the details.

But using interfaces might help (in this case the Payment entity will be polutet with DAL code)... doesn't smell good eighter.

So probably registration of the classes would do the job:

private void SavePaymentType(PaymentType )
{
   if (paymentType == null)
       throw new NotSupportedException("Handle nulls too");
   IClassPersister persister;
   if (!paymentType2Persister.TryGetValue(paymentType.GetType(), out persister))
      throw new ORMException(string.Format("Cannot find persister for {0}", paymentType.GetType().Name))
   persister.Save(paymentType);
}

And somewhere during app startup you could register the PaymentTypes:

paymentType2Persister.Add(typeof(XXXPayment), new XXXPaymentPersistor);
paymentType2Persister.Add(typeof(YYYPayment), new YYYPaymentPersistor);
// etc

So when you'll need to add another payment type you will have to implement persistor for it and register it.
This looks much better then the original code for me.

Cheers.

Dmytrii Nagirniak
Yeah, I feel better with your solution too.
deerchao