What's the best way to separate concerns for this code?

Question

In a previous question one of the comments from Dr. Herbie on the accepted answer was that my method was performing two responsibilities..that of changing data and saving data.

What I'm trying to figure out is the best way to separate these concerns in my situation.

Carrying on with my example of having a Policy object which is retrieved via NHibernate....

The way I'm currently setting the policy to inactive is as follows:

Policy policy = new Policy();
policy.Status = Active;

policyManager.Inactivate(policy);

//method in PolicyManager which has data access and update responsibility
public void Inactivate(Policy policy)
{
    policy.Status = Inactive;
    Update(policy);
}

If I were to separate the responsibility of data access and data update what would be the best way to go about it?

Is it better to have the PolicyManager (which acts as the gateway to the dao) manage the state of the Policy object:

Policy policy = new Policy();
policy.Status = Active;

policyManager.Inactivate(policy);
policyManager.Update(policy);

//method in PolicyManager
public void Inactivate(Policy policy)
{
    policy.Status = Inactive;
}

Or to have the Policy object maintain it's own state and then use the manager class to save the information to the database:

Policy policy = new Policy();
policy.Status = Active;

policy.Inactivate();

policyManager.Update(policy);

//method in Policy
public void Inactivate()
{
    this.Status = Inactive;
}

Answer 1

As a continuation of my original comment :) ... Currently your best bet is the third option, but if things get more complex you could go with the second, while adding facade methods to perform pre-specified sequences:

Policy policy = new Policy();

policy.Status = Active;

policyManager.InactivateAndUpdate(policy);


//methods in PolicyManager
public void Inactivate(Policy policy)
{
    // possibly complex checks and validations might be put there in the future? ...
    policy.Status = Inactive;
}

public void InactivateAndUpdate(Policy policy)
{
    Inactivate(policy);
    Update(policy);
}

The InactivateAndUpdate is a kind of facade method, which is just there to make the calling code a little neater, while still allowing the methods doing the actual work to be separate concerns (kind of breaks single responsibility for methods, but sometimes you just have to be pragmatic!). I deliberately name such methods in the style XandY to make them stand out as doing two things.

The InactivateAndUpdate method then frees you up to start implementing strategy patterns or splitting out the actual implementation methods as command objects for dynamic processing or whatever other architecture might become feasible in the future.