best practice when order of calls in class are important?

Question

I have one class with two important functions:

public class Foo {
    //plenty of properties here
    void DoSomeThing(){/*code to calculate results*/}
    void SaveSomething(){/* code to save the results in DB*/}

}

SaveSomething() uses the results calculated in DoSomeThing().

the problem is that we must not to call SaveSomething() before DoSomeThing() or if that happens the results are not true results. I mean order of calls are important, this is a problem in maintaining the code.(when new one is added to team).

is there any way to manage this?

I think of 3 methods as below

1. throwing exception in SaveSomething() if it called before DoSomeThing()

2. having a bool that are set in DoSomeThing() and SaveSomething() code changes to:

   bool resultsAreCalculated = false;
   void SaveSomething(){
       if (!resultsAreCalculated) {
           DoSomeThing();
           // the resultsAreCalculated = true; is set in DoSomeThing();
           // can we throw some exception?
       }
       /* code to save the results in DB*/
   }
   

3. implementing it Fluent like :

   Foo x = new Foo();
   x.DoSomeThing().SaveSomething();
   

   in this case, it is important to guarantee that this is not happens:

   x.SaveSomething().DoSomeThing();
   

right now, i use the second method. is there any better way or is that enough?

Answer 1

Do and Save methods are not seems like an ordered pair to me. You need to order them only because you don't return the state of the calculation from the Do method. If you write Do method as a method that return the results to the client code, then you can rewrite Save, so it would receive the results as a parameter.

Benefits:

1. You don't need to order methods anymore, because Save method doesn't care how the client got the parameter. It just receives it at that's it.
2. You can unit test Do method more easily, because methods become less coupled.
3. You can move your Save method to another class, if you ever need to write a complex save logic or implement a repository pattern.

Answer 2

How about this?

interface Result {
     void Save();
     SomeData GetData();
}
class Foo {
     Result DoSomething() { /* ... */ }
}

Usage:

myFoo.DoSomething().Save();
//or something like:
var result = myFoo.DoSomething();
if (result.GetData().Importance > threshold) result.Save();

From an outside perspective, this makes a lot of sense. A Result is produced and provides means of being saved, if desired, while the implementation is completely opaque. I don't have to worry about passing this back to the right Foo instance. In fact I can pass the result to objects, that don't even know the Foo instance that created it (in fact the creator should pass on all necessary information for saving to the result upon creation). The result may have a method to tell me, whether it has been saved already, if that is needed. And so on.

This is basically just application of the SRP, although primarily on the interface rather than the implementation. Foo's interface provides means to produce results, Result abstracts means to manipulate results.

Answer 3

Steve McConnel's excellent book Code Complete spends a whole chapter discussing this question. It's Chapter 14 in the second edition.

If the order of statements is important, then it is very good practice to enforce that ordering with data. So rather than

calculateResults();
saveResults();

(storing the results in instance variables) write

Results r = calculateResults();
saveResults(r);

It is then much harder to try to save the results before they are calculated. There is a clear indication of which the expected order is.

Answer 4

One option to help avoid user error is to make it clear by passing a variable. By doing this, it raises a flag for the user that they need to get the results (i.e. DoSomething()) before calling SaveSomething(...).

results = DoSomething(); // returns the results to be saved
SaveSomething(results);

Answer 5

I like Anas Karkoukli's answer, but another alternative is a state machine.

public class Foo {

    private enum State {
        AwaitingDo,
        AwaitingValidate,
        AwaitingSave,
        Saved
    }

    private State mState = State.AwaitingDo;

    private void Do() {

        // Do something
        mState = State.AwaitingValidate;
    }

    private void Validate() {

        // Do something
        mState = State.AwaitingSave;
    }

    private void Save() {

        // Do something
        mState = State.Saved;
    }

    public void MoveToNextState() {
        switch (mState) {
            case State.AwaitingDo:
                Do();
                break;

            case State.AwaitingValidation:
                Validate();
                break;

            case State.AwaitingSave:
                Save();
                break;

            case State.Saved:
                throw new Exception("Nothing more to do.");
                break;
        }
    }
}

It's a bit slap-dash, but you get the idea.

The problem with Anas' answer is that all of the functions are executed as a single step, which means you can't get to the intermediate stages of the object. A state machine forces developers to follow the workflow, but each at each stage of the workflow they can examine the properties of the object before moving on to the next.

Answer 6

Expanding upon Levinaris' answer (+1 if I had the rep), you could alternatively have a Save() method on the results object returned from the DoSomthing() method. So you would get something like this:

var obj = new Foo();

// Get results
var results = obj.DoSomething();

// Check validity, and user acceptance
if(this.AreValidResults(results) && this.UserAcceptsResults(results))
{
    // Save the results
    results.Save();
}
else
{
    // Ditch the results
    results.Dispose();
}

Obviously this approach would require that the returned results object is either a generic type that handles the saving/disposing of results, but also contains the generic results; or it would need to be some form of base class that specific result types could inherit.

Answer 7

Ideally methods that need to follow a certain order in execution denote, or imply the need to implement, a workflow of some sort.

There are a couple of design patterns that support enforcing workflow-like linear execution order, such as the Template Method Pattern, or Strategy.

To Take the Template Method approach, your Foo class will have an abstract base that defines the order of executing Do() and Save(), something like:

public abstract class FooBase
{
    protected abstract void DoSomeThing(); 
    protected abstract void SaveSomething();
    public void DoAndSave()
    {
        //Enforce Execution order
        DoSomeThing();
        SaveSomething();
    }

}

public class Foo : FooBase
{
    protected override void DoSomeThing()
    {
        /*code to calculate results*/
    }

    protected override void SaveSomething()
    {
        /* code to save the results in DB*/
    }
}

This way You class consumers will only have access to DoAndSave() and they will not infract the order of execution you intended.

There are another patterns that deals with workflow / state transition type of situations. You can refer to Chain of Command, and State Patterns.

In response to your comment: This follows the same Template idea, you add another step in your template, imagine you want to validate the results before saving, you can extend your template to become:

public abstract class FooBase
{
    protected abstract void DoSomeThing();
    protected abstract void SaveSomething();
    protected abstract bool AreValidResults();
    public void DoAndSave()
    {
        //Enforce Execution order
        DoSomeThing();

        if (AreValidResults())
            SaveSomething();
    }

}

And of course for a more elaborate workflow I referred you to the state pattern at the end of my original answer, you can have a more granular control over the transition condition from one state to another.