8

I'm working with a client's API that has multiple different 'services' (around 10 so far), each one imports as its own namespace. Part of their standard API call pattern involves returning an array of error messages:

public class Error {
    public String ErrorMessage {get;set}
    public int errorNumber {get;set}
    ..etc
}

I've been trying to clean up and unify our handling of those messages. I tried to make a single function to handle them, eg:

void CheckErrors(List<Error> errors) {
    if(errors != null && errors.Count() > 0) 
        throw new Exception(errors.First().ErrorMessage);
}

(the actual function is more complex but that gives the general idea)

However, it turns out that every one of their services has its own (identical) definition of this Error class. In C++ I could just template this function and it would work fine, or in a dynamic language it'd just work, but in C# I haven't been able to find a way to do this without making 10+ copies of the same function, each with a different namespace on the Error type.

In my own code I could just add an interface to these classes, but since it's not my code I don't think you can do that in C#? I could make a wrapper class that inherits from each of these and implements the interface, but I'm still stuck with duplicate code for every namespace/service.

Is there any cleaner way to handle this?

zacaj
  • 1,987
  • 1
  • 19
  • 39
  • 2
    How are you referencing these services? If it's by using visual studio's service reference, you can use [the solution I've got for my (similar, not sure if it's a dupe) question.](https://stackoverflow.com/questions/47315409/how-to-avoid-code-repetition-using-a-3rd-party-web-service-without-using-dynam) – Zohar Peled May 23 '18 at 15:46
  • 3
    Could you use `List` ? – Magnus May 23 '18 at 15:46
  • 4
    You can create an `ErrorWrapper` class, with a constructor for each of the source error types, and then use that unified error wrapper throughout your code. –  May 23 '18 at 15:49
  • you can create a unique type as a shared nugget package, or deserialize them to the same classe in your c# code – Boo May 23 '18 at 15:58
  • If the API was generated (e.g. a service reference), you might be able to merge the types during the generation process. See [this question](https://stackoverflow.com/questions/40173147/service-reference-why-reuse-types-in-referenced-assemblies) and this [article on sharing types between client and server](http://blog.walteralmeida.com/2010/08/wcf-tips-and-tricks-share-types-between-server-and-client.html) – John Wu May 23 '18 at 17:12
  • Personally, I would go with the dynamic approach below, however, all of them could potentially have some performance implications. Is performance likely an issue in this area? Or are errors relatively rare? – Reginald Blue May 23 '18 at 19:41

4 Answers4

8

You could consider using a late binding solution either using reflection or dynamic. Both have the same drawback: you loose compile time type safety but if its a very isolated and contained piece of code it should be tolerable:

  • Reflection

    void CheckErrors(List<object> errors) {
    if(errors != null && errors.Count() > 0) 
    {
        var firstError = errors.First();
        throw new Exception(
            firstError.GetType()
                      .GetProperty("ErrorMessage")
                      .GetValue(firstError)
                      .ToString()); }

  • Dynamic

    void CheckErrors(List<dynamic> errors) {
    if(errors != null && errors.Count() > 0) 
        throw new Exception(errors.First().ErrorMessage); }
InBetween
  • 32,319
  • 3
  • 50
  • 90
7

Bear with me... you may need yet another Error class that is identical in properties to their Error class, but that you define in your namespace.

Your method CheckErrors uses your definition of Error.

Finally, you can use AutoMapper to map between their Error types and yours. This is pretty much exactly what AutoMapper is designed for. Since all your contracts are identical, the AutoMapper configuration should be trivial. Of course, you incur some runtime expense of mapping, but I think this would lead to the cleanest statically typed solution given that you can't change their interfaces.

The AutoMapper config + usage will look something like this:

//See AutoMapper docs for where to put config, it shouldn't happen on every call
var config = new MapperConfiguration(cfg => 
{
    cfg.CreateMap<TheirApi.Error, MyNamespace.MyErrorDefinition>();
}
var mapper = config.CreateMapper();

MyErrorDefinition myErrors = mapper.Map<List<MyErrorDefinition>>(listOfTheirErrorObjects);
CheckErrors(myErrors);
p e p
  • 6,593
  • 2
  • 23
  • 32
  • That is an overkill IMHO – Optional Option May 23 '18 at 16:26
  • I would do the same without the AutoMapper thing coding myself a simlpe factory that given an API error class returns my error class but anyway looks like a valid solution to me. – Ignacio Soler Garcia May 23 '18 at 16:31
  • Definitely worth consideration - it really depends on what OP is writing here. Maybe you really don't want that dependency because you're building something where introducing it is a real concern. Or maybe you just need these stupid error classes to do what you want them to do instead of writing not 1 but 10 sets of mapping code :). "AutoMapper is a simple little library built to solve a deceptively complex problem - getting rid of code that mapped one object to another. This type of code is rather dreary and boring to write, so why not invent a tool to do it for us?" – p e p May 23 '18 at 16:34
  • Bear in mind the OP would have to call `cfg.CreateMap()` a total of ten times, if there are ten versions of the third party `Error` class. But AutoMapper does remove the need to write code property by property, which may help if there are a lot of properties. If there are just ErrorNumber and message then I wouldn't bother with the mapper. – John Wu May 23 '18 at 19:31
4

Another way is to use lambdas:

    void CheckErrors<T>(IEnumerable<T> errors, Func<T,string> getMessage)
    {
        if (errors?.Count() > 0) throw new Exception(getMessage(errors.First()));
    }

Then call it like this:

CheckErrors(errors, (e) => e.ErrorMessage);
Optional Option
  • 1,521
  • 13
  • 33
  • 1
    Fixed it with lambdas. – Optional Option May 23 '18 at 16:25
  • 1
    cool idea, but I feel like this is still duplicating code all over the place – zacaj May 23 '18 at 16:49
  • If you like automapper solution then for each line mapping "their" exception to "your" exception you could just create a function CheckError(IEnumerable errors) ( CheckError(errors, (e) => e.ErrorMessage); }. It is the same +1 line for each "their" type but here you get type checking by compiler and no extra dependency. – Optional Option May 23 '18 at 19:02
0

I would define my own Error class that has a constructor that accepts any error object from your vendor and converts it. For example:

public class Error
{
    public string Message { get; private set; }
    public int ErrorNumber { get; private set; } 

    public Error(object vendorError) 
    {
        var t = vendorError.GetType();
        foreach (var source in t.GetProperties(BindingFlags.Instance | BindingFlags.Public))
        {
            foreach (var dest in typeof(Error).GetProperties(BindingFlags.Instance | BindingFlags.Public))
            {
                if (dest.Name != source.Name) continue;
                if (dest.PropertyType != source.PropertyType) continue;
                dest.SetValue(this, source.GetValue(vendorError, null));
            }
        }
    }
}

Then when you have an error list from your third party library, you can convert it using LINQ:

var myErrorList = vendorErrorList.Select( e => new Error(e) );

And now you can access the properties per normal.

See my example on DotNetFiddle

John Wu
  • 50,556
  • 8
  • 44
  • 80