Cancelling an order with a side-effect of logging the operation

I have a data access layer method to cancel an order. This operation should have an associated "operation history" entry.

My first (naïve) implementation looks like this:

public void CancelOrder(int orderMastId, string reason)
{
    string order_id = orderMastId.ToString();
    var order = this.db.Commandes.Where(c => c.Order_Mast_Id == order_id);

    if (order == null || order.Count() == 0)
    {
        throw new EntityNotFoundException();
    }

    Parallel.ForEach(order, commande =>
        {
            string previousStatus = commande.Status;
            if (previousStatus != ShippingStatus.Printed)
            {
                // this is what marks the order as cancelled (legacy DB power!)
                commande.Status = ShippingStatus.Printed;

                // this table stores many operations on the order (status change, etc)
                var hist = new DB.Operations_History
                           {
                               Commande_id = commande.Commande_id,
                               Created_Date = DateTime.Now,
                               Feedback = reason,
                               Final_Shipper = string.Empty,
                               Final_Status = ShippingStatus.Printed,
                               Information = "API",
                               Initial_status = previousStatus,
                               Operation_id = 0,
                               Shipping_Info = string.Empty,
                               Shipping_Reference = string.Empty,
                               User_id = 0
                           };

                this.db.Operations_History.Add(hist);
            }
        });

    this.db.SaveChanges();
}

But this is a clear side-effect and in violation of single responsibility principle.

I'm guessing there exist a design pattern for this kind of thing, but I don't know which.

For instance, I could decide that the history entry is a kind of logging, and use (for inst

-
Cancelling?


I have a data access layer method to cancel an order.

All I see is code that creates a history-entry for each order you have in your collection. Please forgive and correct me if I'm wrong but if that is the cancellation, I don't see any violation of a single responsibility principle.

-
Consistency

I see you declaring your variables explicitly and implicitly. Use var everywhere you can or declare them all explicitly. It's just a personal choice, although I'd prefer using var wherever I can.

-
Naming

Field names use camelCase. The name ´order_id´ is not bad as it describes well what it stands for, but orderId would be more correct as it follows the capitalization styles of Microsoft. Since order is a collection of items, rename it to orders, order would imply you're only expecting one item.

-
Code

Checking if an IEnumerable collection has any items, can be done using the Enumerable.Any() method. This is faster than using the Count() method. Let's say the collection has 3 items. Any() will return when it finds the first item whereas Count() will iterate the whole collection. But, if you have a collection that has a Length or Count property, use that for performance.

Since you also do a null-check, I would create an extension method that combines the two. I've written this one for myself which I use often:

public static class Extensions
{
    public static bool AnySafe(this IEnumerable coll)
    {
        return coll.AnySafe(null);
    }

    public static bool AnySafe(this IEnumerable coll, Func predicate)
    {
        if(coll == null)
            return false;

        return (predicate == null) ? coll.Any() : coll.Any(predicate);
    }
}

//Usage:
if (!order.AnySafe())
    throw new EntityNotFoundException();

StackExchange Code Review Q#71004, answer score: 3