C# interface inheritance with explicit typecast

Question

I have a service for creating, saving and sending different types of orders where some types of them will be able to carry attachements. The service will send orders to another external service by using IExternalService which is used by several other services with different external endpoints.

IExternalService contains a getter for a external IRepository which is used to send orders to external services.

I've created a new interface for those repositories which will be adding attachements IRepositoryWithAttachement.

I'm providing some sample code below where i left out unimportant stuff:

interface IRepository //Standard repo used by different external services
{
string Create(Order order);
void Update(Order order);
}

Orders with attachements use

interface IRepositoryWithAttachement : IRepository //attachable repo
{
string AddFile(Attachement file);
void UpdateFile(Attachement file);
}

Repo that must send attachements aswell as orders

public class Repository : IRepositoryWithAttachement {...}

Service used by many implementations of external services

interface IExternalService 
{
 string Name { get; }
 ....
IRepository Repository { get; }
}

Main service class for creating, saving and sending orders

public class OrderService
{
public string Create(Order order)
{
...
IExternalService eService = _externalServices.GetExternalService(id);

try
        {                
         eService.Repository.Create(order);
        }
        catch (Exception e)
        {
            ....
        }
 ...
}

Now this particular ordertype will be adding attachments and when it gets the repository with IExternalService it will get an IRepository back and trying to call eService.Repository.AddFile(file) but the AddFile method doesn't exist because the return type is IRepository which i want. But my IRepositoryWithAttachement is extending IRepository so i got confused how i would reach it and i managed to do this:

 public string AddFile(Attachement file) {

 IExternalService eService = _externalServices.GetExternalService(id);

        try
        {                
                ((IRepositoryWithAttachement ) eService .Repository).AddFile(file); 
        }
        catch (Exception e)
        {               
           ...
        }
 }
}

Question Am i doing this wrong or is it an ugly solution to my problem of getting hold of the addfile method by typecasting?

Consider composition over inheritance. http://stackoverflow.com/questions/49002/prefer-composition-over-inheritance — chiaboy, Mar 27 '15 at 11:28
The question is: Why do you seperate `IRepository` and `IRepositoryWithAttachement`? Are there realy classes that only implement `IRepository` and not `IRepositoryWithAttachement`? If so, than you need to cast AND check if the cast was succesfull (suggestion: use keyword `as`), because the property `Repository` could contain an instance which does NOT implement `IRepositoryWithAttachement`. — Martin Mulder, Mar 27 '15 at 12:14
Yes sveral classes implements it. I separate mostly because IRepository contain some standard methods used by all implementations. My option was to add the new methods to IRepository or to create IRepositoryWithAttachement and either make this interface a separate interface for classes to use together with IRepository or make it extend IRepository. And thanks i will make the check for the type. — codealot, Mar 27 '15 at 12:44
Don't use `try/catch` intentionally. It is bad practice. Use `if(typeof(IRepositoryWithAttachement).IsAssignableFrom(eService.Repository)){ .. }` instead. Better yet `var repo = eService.Repository as IRepositoryWithAttachement; if(repo!=null) { .. }` — John Alexiou, Mar 27 '15 at 20:19

score 1 · Accepted Answer · edited May 23 '17 at 10:24

The two biggest issues I see are that a) you seem to be using exception handling to protect against repositories that don't implement the interface you need, and b) you are catching Exception, rather than InvalidCastException and/or other specific exceptions which you can anticipate and handle correctly.

IMHO, a better implementation would look something like this:

public string AddFile(Attachement file) {
    IExternalService eService = _externalServices.GetExternalService(id);
    IRepositoryWithAttachement repository = eService.Repository as IRepositoryWithAttachement;

    if (repository == null)
    {
        // report error in some appropriate way and return, or throw an
        // _informative_ exception, e.g.
        //     new NotSupportedException("repository does not support attachments")
    }

    repository.AddFile(file); 
}

Even better would be to categorize your available repository IDs and restrict access according to capabilities so that the AddFile() method is never called in the first place unless you know that the repository implements the necessary interface. Then you can safely cast without ever having to worry about an exception being thrown.

Unfortunately, without a good, minimal, complete code example to clearly illustrate the question, it would be hard or impossible to offer advice any more specific than the above with any assurance of relevance. It is entirely possible that there's a better approach available than what you're using now, but without more context it's not really possible to say what that would be.

The exceptionhandling part of the code was uncomplete i only hacked it in fast to show that some kind of exception handling will be done so it's my bad that this was unclear. Exceptions I want to catch are those casted from within AddFile()(which will send to the external service). My core problem was verifying that i was getting hold of the repository in the "correct" way when using inheritance, to which you gave me a better solution through "as IRep...". — codealot, Mar 30 '15 at 13:29

C# interface inheritance with explicit typecast

1 Answers1