C# – Should the method return void or a bool to indicate it was successful

cdesign-patterns

I am integrating an ERP with a 3rd party reseller.
When we finish processing orders that came from the 3rd party, we send back 2 API calls:

  1. Set the status of the order to ReadyToShip whereby we pass shipment information.
  2. Set the status of the order to Shipped.

I am unsure what the return type of this method should be. I am using exceptions to notify the calling code that there was a problem in setting the status of these orders. Because of this, should my method ExportShipments simply return void? Or should I not being returning void, but perhaps a bool or even a custom return object?

My code looks like this, currently using bool, however I think the use of the bool is not really doing anything here as its really never going to return false (because it will raise an exception):

    public bool ExportShipments(int iconicOrderId, string shippingProvider, string trackingNumber)
    {
        TheIconicModels.Order order = orderRepository.GetOrderById(iconicOrderId);
        bool setStatusToReadyToShipResult = true, setStatusToShippedResult = true;
        if ( order.Statuses.Status == OrderStatusConstants.PENDING)
        {
            setStatusToReadyToShipResult = orderRepository.SetStatusToReadyToShip(order, shippingProvider, trackingNumber);
        }
        if ( order.Statuses.Status == OrderStatusConstants.READY_TOS_SHIP)
        {
            setStatusToShippedResult = orderRepository.SetStatusToShipped(order);
        }

        return setStatusToReadyToShipResult && setStatusToShippedResult;
    }

    public bool SetStatusToReadyToShip(Order order, string shippingProvider, string trackingNumber)
    {
        var parameters = new List<AbstractParam>() {
            new OrderItemsParam(order),
            new ShippingProviderParam(shippingProvider),
            new TrackingNumberParam(trackingNumber),
            new DeliveryTypeParam("dropship")
        };

        TheIconicApiResult result = this.apiService.SendPostRequest("SetStatusToReadyToShip", parameters, String.Empty);
        var jsonResponse = JsonConvert.DeserializeObject<RootObject>(result.ResponseBody);

        return jsonResponse.SuccessResponse != null;
    }

I need to know if the operation was successful as at a higher level, I mark this shipment as sent.

Ideally, what should my method be returning? I'm thinking void and just catching any exceptions from a higher level but would appreciate some advice.

Best Answer

I would also like to mention the idea of Command Query Separation

Ideally,

  • Queries return a result (and don't have side-effects)
  • Commands change the state of a system.

As it seems to me that your methods are essentially commands, I would have them have a void return signature and raise exceptions on errors.

Than again, even if your queries encounter errors, they too should raise exceptions and not "return" something indicating an error. Programmers sometime use null to signify that a query failed, but I think that is ill advised.

Related Topic