Duplicated code in web service consumption

Question

I have a set of classes that abstract away calls to a set of web services. I have 6 classes in this particular group, 4 of which contain a simple function that, while small, is still duplicated. What happens is that if some exception or business rule violation happens in the service, they package it up as a fault object in the response. Otherwise, the fault object is null. As a result, the functions simply check to see if that object is not null and, if so, take the reason provided and throw an exception up the chain. So in one class, it might look like

private void ThrowIfContainsFault(AlphaResponse response)
{
    if (response.Fault != null)
    {
        throw new WhateverException(response.Fault.reasonText);
    }
}

And then the other classes, the response object will be of a different type, but the Fault property is the same and the block of code is the same.

private void ThrowIfContainsFault(BravoResponse response)
private void ThrowIfContainsFault(CharlieResponse response)
private void ThrowIfContainsFault(DeltaResponse response)

(Note: These class names are changed for readability, there is no common ancestor for these response objects.)

My first thought is that I could simply change the functions to receive the Fault object directly and forget the response object, and that's valid. But my overall concern is that while these methods literally do the same thing, the classes themselves are not particularly related, so introducing a common base hierarchy for a single (protected) method does not strike me as a suitable fit.

Here's my throwaway thought that I'm not even sure I like:

Define an empty interface that each class can "implement," then define an extension method against that interface.

internal interface IFaultThrower
{        
}

internal static class IFaultThrowerExtension
{
    internal static void ThrowIfNotNull(this IFaultThrower thrower, Fault fault)
    {
        if (fault != null)
        {
            throw new WhateverException(fault.reasonText);
        }
    }
}

This would allow the related classes to get rid of their own private methods and invoke this common one.

 // this.ThrowIfContainsFault(response);
 this.ThrowIfNotNull(response.Fault);

Again, not sure I like it, but I'm also not sure I like the same general idea creeping into multiple classes, either.

Answer 1

I think it is a bad idea to introduce an empty interface to just add an extension method to it. You should think about an interface as a contract and not merely a way to introduce syntax sugar :)

I believe instead of playing with extension methods it is enough to introduce a separate, helper, internal class whose only purpose would be to throw an exception if fault object is null:

    internal class FaultResponseChecker
    {
        internal static void ThrowIfNotNull(Fault fault)
        {
            if (fault != null)
            {
                throw new WhateverException(fault.reasonText);
            }
        }
    }

If you, as I, don't like static methods, then you can use IoC container to inject IFaultResponseChecker into the constructor of every class.

Answer 2

If AlphaResponse, BravoResponse etc all inherit an interface "IResponse" then you could do this.

internal static class FaultThrowerExtension
{
    internal static void ThrowIfContainsFault<T>(this T response) where T : IResponse
    {
        if (response.Fault != null) throw new WhateverException(fault.reasonText);
    }
}

Update : After reading your comment that they don't have a common Interface you could then do this. This is a lot more messey though.

internal static class FaultThrowerExtension
{
    internal static void ThrowIfContainsFault<T>(this T response) where T : class
    {
        var type = typeof(T).FullName;

        foreach (var propertyInfo in PropertyInfoCache[type])
        {
            if (propertyInfo.Name == "Fault")
            {
                var fault = propertyInfo.GetValue(value, null);
                if (response.Fault != null) throw new WhateverException(fault.reasonText);
            }
        }
    }
}