3

This is not something I would ever normally do, but I have a situation where some existing legacy code is being reused in a new application. The code is shared and needs to be used by both the legacy and new application. The legacy application is a reporting application and errors are reported as messages no differently to informational messages in the UI. The new application actually executes actions based on these calculations: so errors are now critical to program execution.

We have hundreds of business logic methods that sort a person into a series of buckets, where the buckets are contained in a dictionary (key is bucket number, value is true if they are in that bucket according to the calculation):

public Dictionary<int, bool> Calculation(IPerson person, out string message)
{
    try
    {
        var buckets = new Dictionary<int, bool>();

        // Do logic to determine what buckets the person is in
        // message is used by the legacy application UI...can be set to anything
    }
    catch(Exception)
    {
        message = "Some non-consistent error message";
    }

    return buckets;
}

In each method there is an all encompassing exception block that swallows the exception (yes...I know).

For the purposes of these calculations an exception failure does not mean that the whole calculation should be discarded: if there is an exception and the person doesn't end up in all the buckets they should we don't care. The message is used to communicate back to the UI that for this person, the whole calculation may not have completed and that is considered enough. Message may also contain non error information...its not specifically an error message and when there is an error there is no consistent text to look for.

With the new application, we potentially want to take a different execution path if there was an exception: at the very least we want to consistently detect and display errors to the UI in an obvious and distinct fashion, not just as whatever text was assigned to the message variable. Also we would like to log the exception details in this new application.

I am not allowed to make a change that will alter the behaviour of the legacy application, but I can refactor the method signatures. I am proposing that the exception be caught as it is now, but to return it is an out parameter:

public Dictionary<int, bool> Calculation(IPerson person, out string message, out Exception exception)
{
    try
    {
        var buckets = new Dictionary<int, bool>();

        // Do logic to determine what buckets the person is in
        // message is used by the legacy application UI...can be set to anything
    }
    catch(Exception ex)
    {
        message = "Some non-consistent error message";
        exception = ex;
    }

    return buckets;
}

I know this is totally non-standard and absolutely not the way exceptions are intended to be used, but in a situation where we want to report that an exception occurred that was handled...why not pass back the exception itself? keep in mind also I can't change the behaviour of the legacy application.

Thoughts? Is this still a terrible idea? Is there a better way of doing this given the constraints?

2

I think that the requirement not to change the behavior of the legacy application is pretty restrictive. Further, I completely agree with you that the right thing to do is just throw the Exception and catch it where it needs to be caught, by the UI.

That said, given the requirement not to change the behavior, I think your solution is a tolerable compromise, given the situation. It does what you need it to do for the reasons you've stated. The only change I would make is to throw both message and Exception into it's own class, e.g.

public Dictionary<int, bool> Calculation(IPerson person, out ErrorResult result) {
    try {
    // snip
    } catch (Exception ex) {
        result.message = "Some non-consistent error message";
        result.exception = ex;
    }
}

Mostly because of the principle of "related objects ought to be bound together in the same class".


You may even want to consider wrapping both the Dictionary and the ErrorResult class into one, larger class to remove the out parameter. In fact, I would probably do something like:

public class Result {
    Dictionary<int, bool> result;
    List<ErrorResult> errors;
}

This way you can keep track of all the possible thrown exceptions and their messages. Your method signature would then become:

public Result Calculation(IPerson person) {
1

How about a different change to the methods:

Gut all the existing methods, move the code into new methods. The existing methods contain only the exception-swallower and otherwise call the new methods to do the work. This leaves the legacy form that you can call if you want it swallowed or the new form if you want it to throw when it should throw.

This requires less in the way of actual modification to the code (although it does require more moving of code) and as far as I'm concerned is much cleaner than your approach.

  • The problem is that both applications want those exceptions to be caught and handled: in both cases we still want that Dictionary<int,bool> result returned if an exception was thrown, its just that the second application wants to know for sure if an exception was thrown and what it was so it can report that the action it is taking is based on a calculation in which errors were encountered and so the action should be reviewed. The first application doesn't report errors specifically, just whatever happens to come back in the message string. – MrLane Jul 30 '15 at 4:40
  • Otherwise I like your suggestion of using a wrapper class for legacy compatibility ;) – MrLane Jul 30 '15 at 4:41
0

I think your best bet is to simply use the catch block to log the exception and any other information that will help you trace the error, including a stack trace. This satisfies your callers, who don't have to change their method signatures, and further satisfies the developers, who now have a way to diagnose problems involving thrown exceptions.

  • The problem is that merely logging the exception does not communicate the error back to the calling code. In fact in this shared code I cant use a logging framework as the two applications use different frameworks. – MrLane Jul 30 '15 at 2:42
  • If we're going to break the API, then why not just rethrow, and require your callers to handle the exception? Or not handle the exception at all, let your callers handle the thrown exception directly? – Robert Harvey Jul 30 '15 at 2:43
  • We have control over all of the code, so breaking the interface is fine...it is simple enough to refactor this: the legacy application can simply ignore the new out Exception parameter passing a null Exception variable but not use it. Breaking the behaviour is not acceptable. The legacy application does not expect that an exception will ever be thrown from the Calculate method and isn't interested in exceptions that are thrown and caught in there. – MrLane Jul 30 '15 at 2:49
  • If a person gets allocated into buckets 1 and 3, then an exception was thrown before they could potentially get allocated into other buckets by the Calculate method, the legacy application does not care. The new application also doesn't care and will still use the half determined result, but does want to report that a distinct error happened: hence passing back the exception object to indicate something did go wrong.. – MrLane Jul 30 '15 at 2:55
  • I guess it's as good a solution as any. I've never seen exceptions passed around this way, but why not. It's an object, like any other. – Robert Harvey Jul 30 '15 at 4:10

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.