Another Vending Machine implementing State Pattern

Question

I previously asked for a review for a Vending Machine implementing State Pattern in this linked post.

And received an answer from @radarbob. The code below is my revised attempt based on some of @radarbob's answer.

Notes / Changes from my original attempt:

• VendingMachine internals no longer public
• protected properties instead
• Abstract VendingMachine base, with Concrete VendingMachine with virtual methods allowing for extensibility
• Products/Inventory by composition
• No Writelines in Domain

Original Question:

I was asked to implement a Vending Machine in a recent interview coding challenge. My attempt at a solution is based on the state pattern.

I would like to get some code reviews on this bearing in mind I would have about 40 minutes to implement this in the interview scenario. When reviewing I would like you to consider how I have done in relation to SOLID principles.

And also consider that the requirements from the interview are:

• Create a vending machine library
• Consumers of the library can list products
• Ability to select product
• After product selected, ability to insert coins
• When enough coins are submitted, product and change dispensed
• Ability to cancel
• Dont worry about stock levels
• Only one consumer at a time
• Think about extensibility

public abstract class VendingMachineBase
{
    protected IState State;
    protected List<Product> Products;
    protected int MoneyInserted;
    protected int ProductChosen;

    protected VendingMachineBase(List<Product> products)
    {
        _ = products ?? throw new ArgumentNullException(nameof(products), "You must provide a product inventory");
        Products = products;
    }

    public void ChangeState(IState state)
    {
        State = state;
    }

    public List<Product> ListProducts()
    {
        return Products;
    }
}

public interface IState
{
    (Product, int) DoWork();
}

public class VendingMachine : VendingMachineBase
{
    public VendingMachine(List<Product> products) :base(products)
    {
        MoneyInserted = 0;
        State = new AwaitingChoice(this);
    }

    public virtual void MakeChoice(int productId)
    {
        ProductChosen = productId;
        State.DoWork();
    }

    public virtual int GetChosenProductId()
    {
        return ProductChosen;
    }

    public virtual void InsertMoney(int amount)
    {
        MoneyInserted += amount;
        State.DoWork();
    }

    public virtual int GetMoneyInsertedAmount()
    {
        return MoneyInserted;
    }

    public virtual (Product, int) DispenseItem()
    {
        return State.DoWork();
    }

    public virtual int RefundMoney()
    {
        State = new Canceling(this);
        var (_, refund) = State.DoWork();

        MoneyInserted = 0;

        return refund;
    }

    public virtual void ResetMachine()
    {
        MoneyInserted = 0;
        ProductChosen = 0;
    }
}

public class AwaitingChoice : IState
{
    private readonly VendingMachine _vendingMachine;
    public AwaitingChoice(VendingMachine vendingMachine)
    {
        _vendingMachine = vendingMachine;
        _vendingMachine.ResetMachine();
    }

    public (Product, int) DoWork()
    {
        _vendingMachine.ChangeState(new AwaitingMoney(_vendingMachine));
        return (null, 0);
    }
}

public class AwaitingMoney : IState
{
    private readonly VendingMachine _vendingMachine;
    private readonly Product _productChosen;
    public AwaitingMoney(VendingMachine vendingMachine)
    {
        _vendingMachine = vendingMachine;
        _productChosen = _vendingMachine.ListProducts().FirstOrDefault(x => x.Id == _vendingMachine.GetChosenProductId());
    }

    public (Product, int) DoWork()
    {
        if (_vendingMachine.GetMoneyInsertedAmount() >= _productChosen.Price)
        {
            _vendingMachine.ChangeState(new Dispensing(_vendingMachine));
        }

        return (null, 0);
    }
}

public class Dispensing : IState
{
    private readonly VendingMachine _vendingMachine;
    private readonly Product _productChosen;

    public Dispensing (VendingMachine vendingMachine)
    {
        _vendingMachine = vendingMachine;
        _productChosen = _vendingMachine.ListProducts().FirstOrDefault(x => x.Id == _vendingMachine.GetChosenProductId());
    }

    public (Product, int) DoWork()
    {
        var returningChange = _vendingMachine.GetMoneyInsertedAmount() > _productChosen.Price
            ? _vendingMachine.GetMoneyInsertedAmount() - _productChosen.Price
            : 0;

        _vendingMachine.ChangeState(new AwaitingChoice(_vendingMachine));

        return (_productChosen, returningChange);
    }
}

public class Canceling : IState
{
    private readonly VendingMachine _vendingMachine;

    public Canceling(VendingMachine vendingMachine)
    {
        _vendingMachine = vendingMachine;
    }

    public (Product, int) DoWork()
    {
        var refund = _vendingMachine.GetMoneyInsertedAmount();

        _vendingMachine.ChangeState(new AwaitingChoice(_vendingMachine));

        return (null, refund);
    }
}

public class Product
{
    public int Id { get; }
    public string Name { get; }

    public int Price { get; }

    public Product(int id, string name, int price)
    {
        Id = id;
        Name = name;
        Price = price;
    }
}

class Program
{
    static void Main(string[] args)
    {
        //worker installs vending machine and populates products
        var products =  new List<Product>()
        {
            new Product(1, "Guinness", 3),
            new Product(2, "Tayto", 2)
        };

        var vendingMachine = new VendingMachine(products);

        //Customer uses Vending Machine

        //Choose Product
        vendingMachine.MakeChoice(1);

        //Insert money
        vendingMachine.InsertMoney(1);

        //Not enough, insert more
        vendingMachine.InsertMoney(3);

        var (product, change) = vendingMachine.DispenseItem();
        Console.WriteLine($"Got my {product.Name}, change is {change}.");

        //Choose Product
        vendingMachine.MakeChoice(2);

        //Insert money
        vendingMachine.InsertMoney(1);

        //Ah, no coins left, I will cancel
        var refund = vendingMachine.RefundMoney();
        Console.WriteLine($"Got my refund {refund}.");

        Console.ReadLine();
    }
}

Answer 1

I think it's more usual when using the state pattern for the state to have a set of well-defined methods on it rather than one general method. You can see that your return value for DoWork is barely used.

I'll try to show that using just the first transition: selecting a product. I haven't compiled the code, I've written it straight into CR so apologies for any errors on my part.

internal abstract class VendingMachineState
{
     protected readonly VendingMachine Machine;

     public VendingMachineState(VendingMachine machine)
     {
         Machine = machine;
     }

     public abstract void ChooseProduct(int productId);
}

internal class AwaitingChoiceState : VendingMachineState
{
    public AwaitingChoiceState(VendingMachine vendingMaching): base(vendingMachine) { }

    public override void ChooseProduct(int productId)
    {
        Machine.SetState(new ProductChosenState(Machine, productId));
    }
}

internal class ProductChosenState : VendingMachineState
{
    public int ProductId { get; }        

    public ProductChosenState(VendingMachine vendingMaching, int productId): base(vendingMachine)
    {
        ProductId = productId;
    }

    public override void ChooseProduct(int productId)
    {
        // You might want this to be a no-op, or change the void return type to something more meaningful. 
        throw new InvalidOperationException("Product already selected");
    }
}

Your VendingMachine then just forwards calls to the state:

public void ChooseProduct(int productId)
{
    _state.ChooseProduct(productId);
}

This achieves the decoupling you want in the vending machine. The next step would be allowing the ProductChosen state to accept money and ensuring that the AwaitingChoiceState doesn't allow it.

---

It might also be worth reconsidering what you consider the states of the machine. I think these exist:

1. Initial - transitions to 2. when product is selected
2. ProductChosen - transitions to 3. when enough money has been added
3. ReadyToDispense - transitions to 1. when product is dispensed

Even the need for ReadyToDispense is debatable. I think it could work with just Initial and ProductChosen but the third state keeps each state's responsibility smaller. ReadyToDispense might also reject further input of money for example.

There's no need for a cancelling state for example. It's just a transition back to initial with a side-effect that money should be returned if it has already been added which is the individual state's responsibility.