Temporal Coupling

A friend of mine shared a private repository with me and asked me to review his code.

While reviewing, I came across the following piece of code:

class Import
{
    public function __construct(private IndexDocument $indexDocument)
    {
    }

    public function import()
    {
        $data = $this->indexDocument->prepareData(
            [
                'first_name' => 'Ahmad',
                'last_name' => 'Mayahi',
            ]
        );

        $this->indexDocument->index($data);

        // ...
    }
}

The import method needs to call prepareData before calling index in order to prepare the given data for Elasticsearch.

If you do not prepare the data, your data will be invalid and not ready to be indexed.

This is a bad design because it introduces Temporal Coupling.

Temporal coupling is a kind of coupling where code is dependent on time in some way. It is particularly insidious because it is hard to detect unless you know what you are looking for. (source)

The fix is straightforward: move prepareData into the index method, so index handles both preparation and indexing:

class Import
{
    public function __construct(private IndexDocument $indexDocument)
    {
    }

    public function import()
    {
        $data = [
            'first_name' => 'Ahmad',
            'last_name' => 'Mayahi',
        ];

        // Prepares and indexes the given data
        $this->indexDocument->index($data);

        // ...
    }
}

Additionally, prepareData should be a private method in IndexDocument since it will only be used within that class.

Whenever you see this pattern in your project -- where method A must be called before method B -- refactor it. Otherwise you will end up saving or handling invalid data.

Summary

Temporal coupling -- a design flaw where code depends on methods being called in a specific order, creating a hidden contract that is easy to violate.
The fix -- internalize the dependent method so the public API handles everything in the correct sequence automatically.
Visibility -- methods that exist solely to support another method's contract should be private, not public.

#Summary

Summary