user369117
user369117

Reputation: 825

exceptions and coupling

I have this main-class that receives a queuemessage and then uses a few other classes to do some work. All these other classes use some lower classes themselves, and eventually data is written to a database or send to wcf services.

Based on the result of the lower classes the main-class has to decide wheter to remove the queuemessage, or place it on the queue again, or send it to a deadletterqueue.

If for example a the database is unreachable, the queuemessage can be placed in the queue to try it again later. But if the wdcf service returns that it doesn't accept some data, the message has to be sent to the deadletterqueue.

I have a couple of ways to implement this scenario:

  1. Throw exceptions and only handle them in the main-class.
  2. Throw exceptions but catch them in each calling class. And rethrow a new exception
  3. Return result-objects which indicate error/success-state

These are my ideas about the scenarios:

  1. If one of the lowest classes throws an exception, and the main-class has to handle it, it couples the main-class all the way to the lowest classes. If one of the lowest classes decides to change an exception, I have to change the main-class exception handling.

  2. There is no good way to let upper classes know which exceptions will be thrown from the called class in C#.

  3. This is what i prefer. Every called method can return a result-object, with an enum indicating succes or failure, and the type of failure.

So, my preferred way is option 3, but I don't know if that's architecturally acceptable. Or if there are any better ways.

Code

This is what the code (in simplified form) looks like:

QueueHandler

private static void HandleQueueMessage(Message message)
{
    var deliveryOrder = deserialize(message.body);

    var deliveryOrderHandler = new DeliveryOrderHandler();

    var result = deliveryOrderHandler.Execute(deliveryOrder.PubId);

    switch (result)
    {
        case DeliveryOrderHandlerResult.DeliverySucceeded:
            break;

        case DeliveryOrderHandlerResult.FataleErrorInExternalSystem:
        case DeliveryOrderHandlerResult.MAndatoryDocuhmentTransformationFailed:
            SendDeliveryOrderToDeadletterQueue(deliveryOrder);
            break;

        default:
            deliveryOrder.AbortCount = deliveryOrder.AbortCount + 1;
            ResendDeliveryOrderToQueue(deliveryOrder);
            break;
    }
}

DeliveryOrderHandler

private DeliveryOrderHandlerResult Execute(long pubId)
{
    DeliveryOrderHandlerResult deliveryOrderHandlerResult;

    var transformationResult = GetTransformationResultaat(pubId);

    if (transformationResult == TransformationResult.Success)
    {
        var deliveryResult = DeliverDocumentToExternalSystem(pubId); 

        if (deliveryResult.Status == DeliveryResult.Success)
        {
            SaveDeliveryResult(pubId, deliveryResult);
        }

        deliveryOrderHandlerResult = deliveryResult.Status;
    }
    else
    {
        switch (transformationResult)
        {
            case TransformationResult.NotStarted:
                deliveryOrderHandlerResult = DeliveryOrderHandlerResult.TransformationNotStarted;

            case TransformationResult.Busy:
                deliveryOrderHandlerResult = DeliveryOrderHandlerResult.TransformationBusy;

            case TransformationResult.MandatoryTransformationFailed:
                deliveryOrderHandlerResult = DeliveryOrderHandlerResult.MandatoryTransformationFailed;

            default:
                throw new Exception(--unknown enum value --);
        }
    }

    return deliveryOrderHandlerResult;
}

DeliverDocumentToExternalSystem

pseudo:
- Create Delivery package by reading data from database and transformed files from disk
- Send package to external system

As you can see there's a lot that can go wrong; failed database connections, failed wcf service calls, files not present etc.

I was hoping I could prevent this:

QueueHandler

private static void HandleQueueMessage(Message message)
{
    var deliveryOrder = deserialize(message.body);

    var deliveryOrderHandler = new DeliveryOrderHandler();

    try
    {
        var result = deliveryOrderHandler.Execute(deliveryOrder.PubId);

        switch(result)
        {
           case DeliveryOrderHandlerResult.Success:
              // remove message from queue

           case DeliveryOrderHandlerResult.NotStarted:
              // resent message to queue

           case DeliveryOrderHandlerResult.MandatoryTransformationFailed:
              // send message to deadletterqueue

           case ...
              // handle

           case ...
              // handle

        }
    }
    pseudo catches:
    catch (DatabaseNotFoundexception ex)
    {
        // resent message to queue
    }
    catch (ExternalWcfServiceDownException ex)
    {
        // resent message to queue
    }
    catch (FileNotFoundException ex)
    {
         // send message to deadletterqueue
    }
    catch (...)
    {
        // handle
    }
    catch (...)
    {
        // handle
    }
}

Upvotes: 0

Views: 359

Answers (1)

Oded
Oded

Reputation: 499062

  1. ... it couples the main-class all the way to the lowest classes.

No. It couple your main class to the exception types. Yes, if you change exceptions lower down you will need to change the handling higher up, but the coupling is on the exception level.

3 is not a good option as you can forget to check the returns. You end up with the exact same issue you list in option 1, as if you change the result objects low down, you need to make changes in your main class...

Exceptions are considered a superior option, as you don't have to check to find out if an error occurred.

Upvotes: 1

Related Questions