The dreaded “Save(Foo bar)”

For the last year or so my biggest antagonist has been “Save(Foo bar)”. There is usually a lot of things going wrong when that method turns up. I’m not saying that saving stuff to the persistence storage is a bad thing, but I’m arguing that the only place it should exist is in actually saving data in the persistence/data layer. Don’t put it in your services, don’t put it in your business logic, and for crying out loud; do not base entire behavior around it.

The source of the ill behavior

When developing software a user will quite often tell you that she wants a button that will save the current changes she’s done to the current work. As a good developer we implement this button and make the user happy. After all, their happiness is our salary. In a pure data-entry application this is all good, but as soon as we add just a little bit of behavior we need to start pushing the actual saving far, far away. Consider this screen:

image

In this setting the Save button makes perfect sense. The user changes a couple of things and then commits those changes to the system. Now start to think about more then just storing the values in a database, think about adding behavior to the changes.

Do not throw away the users intent!

When a user makes a change to any of these fields, they do so with an intention. Changing the address might mean that the receiving end of an order has changed. Changing logistics provider might mean that a new one need to be notified. With a save method, the only way for our business logic to know what to do, is to figure it out;

public void Save(Order changedOrder) { if (changedOrder.Address != existingOrder.Address) OrderAddressChanged(existingOrder, changedOrder); if (changedOrder.Status != existingOrder.Status) OrderStatusChanged(existingOrder, changedOrder); if (changedOrder.LogisticServiceProvider != existingOrder.LogisticServiceProvider) LogisticServiceProviderChanged(existingOrder, changedOrder); }

And even worse;

public void OrderStatusChanged(Order existingOrder, Order changedOrder) { if(changedOrder.Status == Status.Cancelled) ......

Basically what we’ve done is throwing away the users intent and capturing of the actual work the user has done. I see this happening a lot in service interfaces (WCF Service contracts with a single Save method), in “Managers” (Let’s not even go there) and in local services.

The code this will generate a couple of releases down the road will smell in so many scents it’s not even funny to joke about. But to get you started, read up on Open/Closed (A SOLID principle).

 

Enough with the bashing, what’s the solution?

The idea that a user can commit their work in a single push of a button is a good one. But that doesn’t mean that the save need to anything else then a button. The actual work under the covers should reflect what the user really wants to happen. Changing a service provider on an order that’s packed should probably issue a message to the old provider not to pick the package up and a message to new to actually do. This kind of behavior is best captured with commands.

Let’s revisit the order form:

image

In this scenario every red box here is actually a intended command form the user. Identified as;

ChangeOrderAddress

ChangeOrderLogisticServiceProvider

ChangeOrderStatus (or actually, CancelOrder, ShipOrder, etc)

Implementing this could either be using the command pattern or methods on the order class. “It all depends” on your scenario on preferences.

So what am I really suggesting?

Make sure that your code reflects the intention of the user. Not the technical details of how the data is persisted. You will save a lot of pain, grief and hard work by just identifying what the user intends, capture that in commands or something similar and execute them in order when the user hit’s save. And most importantly SAVE IS A METHOD ON THE PERSISTENCE LAYER, NOT A BUSSINESS FUNCTION.

2 thoughts on “The dreaded “Save(Foo bar)”

  1. There is more to this than just services.
    There might be multiple reasons why you edit a “customer address” or whatever.

    One reason might be along the lines of what you have described here, the customer have moved and this change should maybe trigger some sort of process, sending emails, redirecting existing orders and so on.

    Another reason might be that there is simply a spelling mistake in the current address.
    This change should probably not trigger the exact same processes as the first change.

    And the only way to deal with this completely is to reflect the intentions for change in your UI also and not only in your service design.
    Be it different views or be it clever UI logic that detects how much of the address has changed.

    //Roger

    • Agreed,

      this post only touched the service design. But from a UI (and really functional perspective) this is why we push for task-based applications instead of “data capture” based application. I will follow up on this :)

Leave a Reply

Your email address will not be published. Required fields are marked *

*

You may use these HTML tags and attributes: <a href="" title=""> <abbr title=""> <acronym title=""> <b> <blockquote cite=""> <cite> <code> <del datetime=""> <em> <i> <q cite=""> <strike> <strong>