Casting and Interfaces Dilema

Question

So I'm using interfaces more these days. But I am coming across a brick wall this time.

Just for context only let me show you the RESTful WCF contract here that I designed to show you how I'm using IPaymentRequest:

[ServiceContract]
public interface IPaymentService
{
    [WebInvoke(Method = "POST", UriTemplate = "/PreAuthorization")]
    PreAuthorizeResponse SendTransaction(PreAuthorizeRequest request);

    [WebInvoke(Method = "POST", UriTemplate = "/Capture")]
    CaptureResponse SendTransaction(CaptureRequest captureRequest);

    ... and so on
}

The implementation of the Service Contract for example has some methods that look like this:

public PreAuthorizeResponse SendTransaction(PreAuthorizeRequest request)
{

   .....
   Processor.SetSettings(request);

 }

(Note/disclaimer on clean code principals. I have better names for stuff like the name SetSettings() but for privacy I've named stuff more simple such as "SetSettings" and "Process" for this Stack post. In reality I have what kind of processor in its class name so just FYI).

Second, let me make you aware that I have a Processor class that is basically kinda like a utility class to do some things such as send the request fields to an outside REST API. And in this class one example of another method I'm setting up is a SetSettings method that I'll set some stuff based on the type of request that comes in. Mostly, I'm going to get the stuff I need from its Transaction property.

public class Processor
{
    private void SetSettings(IPaymentRequest request)
    {
         var someValue = request.Transaction.SomeProperty1;
         ...
    }
}

Now here's what the IPaymentRequest looks like:

public interface IPaymentRequest
{
    string Token { get; set; }
    int ClientID { get; set; }
}

Now here are a couple examples of my domain models (the Models my Service Contract expects to be sent in from client requests) that implement IPaymentRequest:

public class PreAuthorizeRequest : IPaymentRequest
{
    public string Token { get; set; }
    public int ClientID { get; set; }
    public int Amount { get; set; }
    public PreAuthTransaction Transaction { get; set; }
}

public class CaptureRequest : IPaymentRequest
{
    public string Token { get; set; }
    public int ClientID { get; set; }
    public int BankID { get; set; }
    public CaptureTransaction Transaction { get; set; }
}

I'm using IPaymentRequest throughout my WCF service (it's the type that's expected to be sent into my payment service's method contracts) and using these interfaces elsewhere in my service to make some good reuse of methods that these requests can flow through such as SendRequest(IPaymentRequest request), and so on.

Here is the dilema/problem I have:

In methods where I want to reuse the logic for any kind of request that comes in, I end up having to check for what type it is incoming to my methods sometimes in my processor class. So I am having to create a bunch of messy if statements in order to determine and cast the incoming ITransaction in order to start using it in my utility mehtods here.

So lets continue more so I can explain more about my First method SetSettings()

Notice that I need values from the transaction object in the request and to be able to work with properties in that TYPE of request.

Now lets take a look at the CaptureTransaction object for example for a CaptureRequest

public class CaptureTransaction : ITransaction
{
    public string Reference { get; set; }
    public decimal Amount { get; set; }
    public string CurrencyCode { get; set; }
    public CreditCard CustomerCreditCard { get; set; }
}

So as you can see, for each Request Type I have a related concrete Transaction Type that implements ITransaction and holds info that the transaction needs to send over to an external API.

Note: All requests WILL always have a transaction (ITransaction) so I thought it'd be therefore a good idea to maybe throw ITransaction in my IPaymentRequest so something like this:

public interface IPaymentRequest
{
    string Token { get; set; }
    int ClientID { get; set; }
    ITransaction Transaction {get; set; }
}

And here is ITransaction. Every request that comes into our service will require a currency now and in the future so this field was a good candidate/reason to use an Interface:

public interface ITransaction
{
    string CurrencyCode { get; set; }
}

So adding that to my IPaymentRequest now requires me to change the Property Name in my Custom Types to "Transaction", for example:

public class CaptureRequest : IPaymentRequest
{
    public string Token { get; set; }
    public int ClientID { get; set; }
    public int BankID { get; set; }
    public ITransaction Transaction { get; set; }
}

I thought ok fine.

But now if I try to work with Transactions in my utility method, since it's an Interface variable, it has no idea what type of Transaction it is. So I end up having to cast it before I can use it:

private void SetSettings(IPaymentRequest request)
{
     ITransaction transaction;

     if (request is CaptureRequest)
         transaction = request.Transaction as CaptureTransaction;
     if (request is PreAuthorizeRequest)
         transaction = request.Transaction as PreAuthorizeTransaction;

... etc.

     var someValue = request.Transaction.Some1;
     ...carry on and use SomeProperty1elsewhere in this method for whatever reason
}

IMO it just feels strongly like huge code smell. So obviously I am not doing something right or I don't yet know something about Interfaces that I should know...that allows me to use them better here or without so much casting. And just too much casting IMO is bad, performance-wise.

Maybe this is a good case to use Generics instead of interface parameters in methods I want to create for reuse across different types of Concrete Request types (Capture, PreAuth, Void yada yada)?

The whole point here is I want to be able to specify interface params in some methods to make them DRY (don't repeat yourself) / reusable...and then use the concrete type that came in via polymorphism and work with the request instance.

Answer 1

If every request has a transaction, then this is the right way to go:

interface IPaymentRequest
{
    string Token { get; set; }
    int ClientID { get; set; }
    ITransaction Transaction { get; set; }
}

Obviously, to process the custom request, you'll need a custom processor:

class Processor
{
    protected virtual void OnSetSettings(IPaymentRequest request)
    {
    }

    private void SetSettings(IPaymentRequest request)
    {
        // do the common stuff
        // ...
        // set custom settings
        OnSetSettings(request);
    }
}

class PreAuthorizeRequestProcessor : Processor
{
    protected override void OnSetSettings(IPaymentRequest request)
    {
        base.OnSetSettings(request);

        // set custom settings here
        var customRequest = (PreAuthorizeRequest)request;
    }
}

As you can see, this requires a little type casting. Yo can avoid casting with generics, but this brings a complexity in types declaration:

interface IPaymentRequest<TTransaction>
    where TTransaction : ITransaction
{
    string Token { get; set; }
    int ClientID { get; set; }
    TTransaction Transaction { get; set; }
}

class Processor<TRequest, TTransaction>
    where TRequest : IPaymentRequest<TTransaction>
    where TTransaction : ITransaction
{
    protected virtual void OnSetSettings(TRequest request)
    {
    }

    private void SetSettings(TRequest request)
    {
        // do the common stuff
        // ...
        // set custom settings
        OnSetSettings(request);
    }
}

class PreAuthorizeRequestProcessor : Processor<PreAuthorizeRequest, PreAuthTransaction>
{
    protected override void OnSetSettings(PreAuthorizeRequest request)
    {
        base.OnSetSettings(request);

        // set custom settings here
    }
}

Answer 2

Explanation to my comment (how to use visitor pattern in this case):

interface IPaymentRequest
{
    void Process(IPaymentRequestProcessor processor);
}

class CaptureRequest  : IPaymentRequest
{
    public void Process(IPaymentRequestProcessor processor)
    {
        processor.Process(this);
    }
}

class PreAuthorizeRequest : IPaymentRequest
{
    public void Process(IPaymentRequestProcessor processor)
    {
        processor.Process(this);
    }
}


interface IPaymentRequestProcessor
{
    void Process(CaptureRequest request);
    void Process(PreAuthorizeRequest request);
}

Where:

private void SetSettings(IPaymentRequest request)
{
    IPaymentRequestProcessor processor = new PaymentRequestProcessor();
    request.Process(processor);
}

Answer 3

The Visitor pattern is one obvious solution - it allows you to step around the fact that C# can't resolve which subtype of ITransaction you're using at runtime in order to choose a method overload by using a trick called double dispatch. The result of the Visitor pattern is to move the type-specific processing code from a conditional (which can miss cases) to a type definition, which the compiler can enforce the completeness of. The cost, however, is code that bounces around through virtual methods in a way that can be a bit complicated to figure out when you're trying to comprehend it from scratch.

Here's how it works.

ITransaction gains a method Accept(ITransactionVisitor visitor). ITransactionVisitor is an interface which has a Visit method with an override for each ITransaction subclass you want to deal with:

interface ITransactionVisitor {
    void Visit(PreAuthTransaction t);
    void Visit(VoidTransaction t);
    // etc.
}

Then of course you need to implement these methods. Accept is easy, but it does need to be implemented for each implementation of ITransaction. Why? Not just because it's an interface method, but because within that method body the compiler will concretely know the type of the transaction at compile time, so it can choose the right overload in ITransactionVisitor.

public void Accept(ITransactionVisitor visitor) {
    visitor.Visit(this);
}

Then all you need to do is implement an appropriate ITransactionVisitor. One of the advantages of this pattern is that you can implement as many as you like with completely different behaviours, and the ITransaction needs no further knowledge or modification (this is why the visitor is specified with an interface or an abstract class).

public class TransactionProcessorVisitor : ITransactionVisitor { public TransactionProcessorVisitor(/* some suitable context in the constructor so it can do its job perhaps */) { ... } public void Visit(PreAuthTransaction t) { // do stuff } public void Visit(VoidTransaction t) { // do other stuff } }

So yes, the visitor classes have to know about all the types of transaction, but

1. It's not in a giant if statement
2. It's not part of the ITransaction implementations
3. It's compile-time checked - if you add a new ITransaction type and try to feed it through the processor, the compiler will be able to figure out that there's no Visit method for it and throw an error, rather than waiting until runtime which is the best you can do with the if version.

This is not necessarily the best answer, but it's an answer.

Answer 4

First, let me tell you that your SetSettings method is wrong. var doesn't work like that. From there, your entire thread of reasoning is wrong. Add to it fact you are using some kind of "utility methods" and you have recipe for very bad architecture.

First, I would change those utility methods into some kind of full-featured classes with some interface. I'm sure you could create IProcessor interface and have PreAuthorizeProcessor and CaptureProcessor. From there, you either have IProcessor GetProcessor() method on your IPaymentRequest, which then forces each request into being able to have it's own processor. Or you could use a factory to create specific processor for given request via IProcessor CreateProcessor(IPaymentRequest). Here, you could either hard-code the preocessors or use some kind of subscribe mechanism.

Also, using type checking and casting is not wrong as long as it is properly encapsulated, like inside a factory. And using a visitor pattern is not much different from doing a manual type checking. You still get same kind of advantages and disadvantages from both.