Question

Let's say I have a method DoTheThing() which requires the precondition-check CanTheThingBeDone() to return true. The latter method is time consuming as it accesses the database.

I'm finding it hard to find a perfect way to perform the precondition check effectively (i.e. once) while maintaining both readability and API integrity. Observe these examples:

Example 1:

class System() {
    public void DoTheThing() {
        if (!CanTheThingBeDone()) throw new Exception(...);
        // Do it
    }

    public bool CanTheThingBeDone() {
        // Some time consuming code
    }
}

class Consumer() {
    public void SomeLargerOperation() {
        try {  
            system.DoTheThing();
        } 
        catch (Exception) { 
            // Error handling
        }
    }
}

Example 2:

class System() {
    public void DoTheThing() {
        // Do it
    }

    public bool CanTheThingBeDone() {
        // Some time consuming code
    }
}

class Consumer() {
    public void SomeLargerOperation() {
        if (system.CanTheThingBeDone()) { 
            system.DoIt();
        }
    }
}

Both approaches are ok, but both have drawbacks. In Example 1, I'm forcing the consumer to wrap the code in a try-catch for faulty states. These states can be user-triggered so some graceful handling is required. I'm unsure if try-catch for handling user-errors is a good approach.

In Example 2 I'm handing the responsibility of state-checking over to the consumer, opening up for non-descriptive runtime errors. In this example, CanTheThingBeDone() is checked, but when Jimmy enters the team 1 year from now to develop module 2, that might change.

Example 3:

class System() {
    public bool DoTheThing() {
        if (!CanTheThingBeDone()) return false;
        // Do it 
        return true;
    }

    public bool CanTheThingBeDone() {
        // Some time consuming code
    }
}

class Consumer() {
    public void SomeLargerOperation() {
        if (!system.DoTheThing()) ProduceSomeErrorMessage();
    }
}

Now in this example, I avoid try-catch for user errors and prevent the actual Do-code to be executed if invalid state, while ensuring the check-code is run only once. But I feel I'm breaking some naming/coding principle when the method is both performing check and modifying class state (I'm pretty sure it even has a name).

As a fourth example, I could envision a combination of #1 & #2 where both the consumer and DoTheThing calls CanTheThingBeDone, but then we're doing the work twice.

To sum up - I can't seem to find a best approach. Anyone have a better idea than mine, or suggestions on how to tweak one of these approaches for a better result?

Was it helpful?

Solution

There is one rule I like: When in doubt, create an object. C# is an OOP language, not functional after all. So thinking in objects is much better than thinking in methods.

public abstract class ExpensiveOperation
{
    bool wasTested = false;
    bool canBeDone = false;

    protected abstract bool Precondition();
    protected abstract void DoExecute();

    public bool CanExecute()
    {
        if (!wasTested)
        {
            canBeDone = Precondition();
            wasTested = true;
        }
        return canBeDone;
    }

    public void Execute()
    {
        if (!CanExecute())
            throw new Exception("Cannot execute");

        DoExecute();
    }
}

Then you just derive this for your custom operation with precondition. It has many good properties:

  • It encapsulates the operation with it's precondition
  • Ensures the precondition is called and operation cannot be executed if precondition fails
  • Ensures the precondition is called only once no matter what caller does.
  • Give caller option to check if it can be executed if necessary
  • You just supply the code for precondition and operation, no need to create more code
  • Ensures the caller cannot bypass this precondition logic

OTHER TIPS

Use the first example whenever possible. This way it's near impossible to forget the check or to forget to handle the error. Also, such a method may return values without resorting to out-arguments.

Why shouldn't I use #2 or #3? Because it is too easy for a Consumer to ignore the error, e.g. when the programmer is unfamiliar with System's API. The #2 is especially bad because there is no way to find out whether the operation completed successfully.

If you cannot use the exception system for some reason (e.g. you're writing C which doesn't have one, or you are abstaining from exceptions for performance reasons), then I'd suggest a combination of #2 and #3: You return an error code, but if a programmer knows what he's doing, he may omit any error checks. But you should also allow the programmer to explicitly insert the checks beforehand.

Don't think too hard about what an elegant solution would look like: first, try to find a robust approach.

You should use some variant of #1. Consumers cannot be trusted to check rules to see if something can be run before running something. That adds temporal coupling, even if people remember it.

Ways to improve it:

  • Lazy - assuming CanSomethingBeDone can never change during the lifetime of the object, you can lazily initialize it so you don't check it every call; just once on first use.
  • Decorators - You should also probably better separate CanSomethingBeDone from DoSomething, since they are separate responsibilities, which may not change together. A decorator seems like a good approach here. Then you can have a code DoSomething and a decorator that checks the rule before calling its wrapped implementation. This keeps things separate and composable. Though this works best when the consumer shouldn't know about the capability at all.
  • No-Ops - Depending on your usage, it may be preferable to simply do nothing rather than throw an exception if the consumer can't run DoSomething. This simplifies the call site, especially if the capability is exposed and you expect that DoSomething won't usually be called because it won't be clickable.
  • Don't expose it - if you've got a composition/plug-in structure, you might be able to hide the entire command when it is unavailable. This though requires a particular architecture and some other constraints that let you know that the plug-in is unavailable early and constantly.
Licensed under: CC-BY-SA with attribution
scroll top