Design pattern for objects in invalid states

Question

General design pattern for object error state

Consider a simple class Wallet that models a wallet. A Wallet contains a certain amount of Wallet.Cash and it is possible to take money out / put money in.

    public class Wallet
    {
        /// <summary>
        /// Indicates the amount of Cash in the wallet
        /// </summary>
        public double Cash
        {
            get;
            private set;
        }

        /// <summary>
        /// Takes some money out of the wallet 
        /// </summary>
        public void Spend(double amount)
        {
            Cash -= amount;
        }

        /// <summary>
        /// Puts some money into the wallet
        /// </summary>
        public void Fill(double amount)
        {
            Cash += amount;
        }
    }

Certain object states are invalid. Those are: Wallet.Cash is negative, NaN or +/- infinity. We now have the task to take precautions such that transitions into invalid Wallet states do not happen unnoticed. Which of the following approaches are "industry standard"?

Solution 0: Do not perform any checks. Rely on severe framework testing

A (questionable?) approach is to not change the implementation at all and rely on testing objects that instantiate and manage Wallet's. After n tests we consider our code working properly.

This approach has the advantage that it optimizes lines of code and performance since no ressources are wasted on integrity checking.

Solution 1: Throw Exception before the object enters an invalid state

Each method could verify that the operation would not lead to an invalid state before executing it. The implementation for the example above would look something like

    public class Wallet
    {
        /// <summary>
        /// Indicates the amount of Cash in the wallet
        /// </summary>
        public double Cash
        {
            get;
            private set;
        }

        /// <summary>
        /// Takes some money out of the wallet 
        /// </summary>
        public void Spend(double amount)
        {
            if (amount > Cash) throw new Exception("Insufficient cash");

            if (0.0d > amount) throw new Exception("Invalid amount");
            if (Double.IsInfinity(amount)) throw new Exception("Invalid amount");
            if (Double.IsNaN(amount)) throw new Exception("Invalid amount");

            Cash -= amount;
        }

        /// <summary>
        /// Puts some money into the wallet
        /// </summary>
        public void Add(double amount)
        {
            if (0.0d < amount) throw new Exception("You are intending to spend it");

            if (Double.IsInfinity(Cash + amount)) throw new Exception("That's a lot of money in total");
            if (Double.IsNaN(amount)) throw new Exception("Invalid amount");

            Cash += amount;
        }
    }

which effectively more than doubles the lines of code, impacting performance and code maintainability. The advantage however is, that the user can catch the Exception knowing that the Wallet is still in the state it was before the method was called and hence can consider whether the failed call has invalidated the state of the whole process.

Solution 2: Throw Exception after the object has entered an invalid state

Each method assumes the input paramters to be valid and does its job. At the end, it checks whether anything is now broken. An implementation would look something like

    public class Wallet
    {
        /// <summary>
        /// Indicates the amount of Cash in the wallet
        /// </summary>
        public double Cash
        {
            get;
            private set;
        }

        /// <summary>
        /// Takes some money out of the wallet 
        /// </summary>
        public void Spend(double amount)
        {
            Cash -= amount;

            AssertValidObjectState();
        }

        /// <summary>
        /// Puts some money into the wallet
        /// </summary>
        public void Add(double amount)
        {
            Cash += amount;

            AssertValidObjectState();
        }

        private static Exception exInvalidObjectState = new Exception("[Wallet] Invalid Object State");
        private void AssertValidObjectState()
        {
            if(Double.IsInfinity(Cash)) throw exInvalidObjectState;
            if(Double.IsNaN(Cash)) throw exInvalidObjectState;

            if(0.0d > Cash) throw exInvalidObjectState;
        }
    }

which adds a tiny snippet at the end of the class and one additional line to each method. While it does not impact code maintainability, it certainly - depending on the complexity of the object - impacts performance and brings the unfortunate side effect, that thrown Exceptions imply the state of the process to be invalid.

Solution 3: Set Object to an error state and throw on next read

Lastly an orthogonal approach that I have seen will not throw the Exception immediately but rather set the object into an error state and throw it the first time is read from an invalid state. An implementation could look something like

    public class Wallet
    {
        /// <summary>
        /// Indicates the amount of Cash in the wallet
        /// </summary>
        public double Cash
        {
            get
            {
                if (null != exInvalidObjectState)
                    throw exInvalidObjectState;

                return cash;
            }
            private set
            {
                cash = value;
            }
        }
        private double cash = 0.0d;

        /// <summary>
        /// Takes some money out of the wallet 
        /// </summary>
        public void Spend(double amount)
        {
            if (null != exInvalidObjectState)
                throw exInvalidObjectState;

            Cash -= amount;

            AssertValidObjectState();
        }

        /// <summary>
        /// Puts some money into the wallet
        /// </summary>
        public void Add(double amount)
        {
            if (null != exInvalidObjectState)
                throw exInvalidObjectState;

            Cash += amount;

            AssertValidObjectState();
        }

        private static Exception exInvalidObjectState = null;
        private void AssertValidObjectState()
        {
            if (Double.IsInfinity(Cash)) exInvalidObjectState = new Exception();
            if (Double.IsNaN(Cash)) exInvalidObjectState = new Exception();

            if (0.0d > Cash) exInvalidObjectState = new Exception();
        }
    }

which implies that it does not really matter whether an object is in an invalid state as long as we don't obtain any data from it. It makes debugging tough and to me seems just wrong to do but ...

Answer 1

How are responsibilities between classes?

There is no single answer to that question. It's first a question of responsibilities:

• Shall using classes be responsible for verifying if they can do the operation before doing it? Typically, this could make sense for UI classes that shall give an immediate feedback to the user about what he/she is allowed to do. But it might end up in solution 0 if some checks are not performed.
• Shall objects be responsible for doing the verifications for itself ? That's solution 1. This would significantly improve the reliability of your code. But if the using objects does not react on error messages it might continue their transaction script as if everything went fine and cause inconsistencies that could have been prevented.
• Shall it be a shared responsibility between the using and used object ?

There are also some variants of the above:

• Defensive coding and checking the invariants and post-conditions each time. But this would be an overhead in production that could only be justified by highly sensitive mission critical systems.
• you can decide to use exceptions, when the used object should raise an error that suggests potential inconsistencies elsewhere (e.g. taking money that is not in the wallet).

It’s an important design decision that you must thereafter apply consistently. There’s nothing worse that a using object assuming the used object would do the check while the using object assumes that only valid operations would be performed.

What about your solutions ?

Solution 0 is a time bomb. Because errare humanum est and sooner or later you might encounter an inconsisten situation that was not anticipated.

Solution 1 is ok (see above).

Solution 2 and 3 are too late since a good status was lost and the object is no longer used. This is only reasonable as part of defensive coding. Or for value objects that can be lost. Note that solution 3 is common in floating point implementations (i.e.NaN state).

Transaction management

Finally, there’s the problem of overall consistency when a transaction involves operations on different objects. Imagine each operation is valid, except one. For these case you need to elaborate a strategy: shall the transaction be interrupted ? Shall the operations already done be reverted (and is it still possible) ? Shall first all the checks be performed and then all the operations executed ?

This is much more complex and needs to be analysed case by case.

In your example:

• If Cash would be a complex object and the first to be touched when doing Spend(), with solution 1 you could just interrupt the transaction and everything is fine. If an exception is raised, it would be passed over to the wallet's context.

• If the wallet's context would use a Spend() from one wallet to Add() cash to a second wallet, what would you do if the first Spend() succeeded but the Add() would fail (for example if there'd be a maximum cash amount a wallet can take) ? Interrupting is not sufficient since you would lose cash. You'd need to handle the exception and add back the amount to the first wallet.

• What-if the wallets can be addressed concurrently by several processes ? while you're doint one transaction, another session could alter the wallets independently. Should you then lock the wallets when you start the transaction ?

Answer 2

No no no no.

First, stop using floating point numbers to represent base 10 money. Ints work fine if you count pennies and remember to add the decimal point when presenting them as dollars.

Second, don't conflate validation with error handling. Nothing says you have to throw an exception. Exceptions are for exceptional cases. Not for every time things don't go perfectly.

Third, learn what a transaction is. I try to buy something with my wallet that I can't afford now, the result shouldn't be an error report or an exception that breaks my wallet. No the result should be I can't buy that thing right now. This means all state changes proposed in this transaction need to be unwound if either the wallet has too little money or the store is out of stock. Just report that the transaction didn't happen and why.

None of that needs a wallet object that throws exceptions. Just one that tells you the result of trying to withdraw money.

Answer 3

Exceptions thrown for normal object access (or "Solution 1") is known as the general pattern coined in Python as better ask for forgiveness than permission.

This pattern is heavy on the user side because it has responsibility to catch all possible errors you would throw accessing your wallet, but this can be the correct thing to do if it is required to make operations atomic in resource acquisition patterns, such as attempting to read a file on system (instead of "checking the file exists" then "read" where the file could be deleted in between).

If you don't have such kind of system-level worries, then the best and most standard thing you could do is to disallow incoherent states for your wallet by remodeling your API contracts, in such way there is no operation sequence that can lead to an invalid object. If it's not valid, it shouldn't exist.

public class Wallet{
    private double cash;
    public double howMuchCanISpend(){
        return cash;
    }
    public void deposit(double amount){
        amount = max(0, amount)
        cash += amount
    }
    public double withdraw(double amount){
        amount = min(max(amount, 0), cash);
        cash -= amount;
        return amount;
    }
}

Answer 4

I suspect you are suffering from “primitive obsession”. Having the validation code for valid states of cash in the wallet means anywhere else you use cash needs them too. If you create a new class called Cash, you can keep all the validation checks there. I would recommend in the constructor.

I would recommend making an immutable value class called Cash that contains all this validation.

This has the added benefit of making sure your methods’ signatures can’t accidentally get passed other doubles not meant to represent cash.

You can write operators to make them easier to use, so that in the end, in wallet class:

Cash cashInWallet;

void AddMoney(Cash cash) {
    cashInWallet += cash;
}

Answer 5

Solution 1 w/ YAGNI applied:

public class Wallet
    {
        /// <summary>
        /// Indicates the amount of Cash in the wallet
        /// </summary>
        public double Cash
        {
            get;
            private set;
        }

        /// <summary>
        /// Takes some money out of the wallet 
        /// </summary>
        public void Spend(double amount)
        {
            if(Cash - amount < 0) throw new exception("Spending exceeded");

            Cash -= amount;
        }

        /// <summary>
        /// Puts some money into the wallet
        /// </summary>
        public void Add(double amount)
        {
            Cash += amount;
        }
    }

Considerations: Cash doesn't need validation, adding 0 while redundant, isn't necessarily a bad scenario (think account balancing or something), spending more than you have is invalid. A double will exceed it's capacity (framework error) before an infinity is approached.