Last Updated: 2024-04-24

When building the reverse auction for project_s, I basically looped through the results of a complex query chooseAdvisors(n) and sent an email to each of these advisors. The worst nightmare bug was that every one in the database would get an email - i.e. that the n argument would be ignored.

The code was written as follows

<?
// This is the method that the job executer triggers
public function handle()
{
    $advisors = $this->chooseAdvisors(10);
    foreach($advisors as $advisor) {
        $this->contactAdvisor($advisor);
    }
}

private function chooseAdvisors($numberToContact) { }
private function contactAdvisor($advisor) { }

This is a design I regret. For confidence, I wanted to see if chooseAdvisors was returning the right number of entries, depending on the argument given. Unfortunately, as a private method of a job, I could not test it easily (either in unit tests or in the production environment). Therefore I had reduced confidence.

In future I would have made it a public method, placing it on another class if needs be, in order to get a clean and sensible interface.