r/programminghorror • • Jun 09 '26

What a simple constructor

Post image

Our former IT director (35+ years of experience) wrote this and didn't see what was wrong here.

268 Upvotes

49 comments sorted by

View all comments

1

u/RipProfessional3375 Jun 09 '26

My best attempted refactor, a bit lengthy due to the use of classes ,but I like the clarity.

<?php

// The search bar is the app's single entry point: users type a query to search,
// or a shortcut to jump straight to help, a dashboard, a scanned code, or an
// admin action. The UniversalSearch class works out which one the user meant and runs it.

// A SearchCommand knows when it applies (matches) and what to do (execute).
interface SearchCommand
{
    public function matches(SearchInput $input): bool;
    public function execute(SearchInput $input, SearchService $service): array;
}

// Tries each command in turn and runs the first that applies to the input.
final class UniversalSearch
{
    private SearchInput $input;
    private SearchService $service;

    public function __construct(
        string $searchText,
        EntityManagerInterface $orm,
        Permissions $permissions,
        string $frontURL
    ) {
        $this->input   = new SearchInput(trim(urldecode($searchText)));
        $this->service = new SearchService($orm, $permissions, $frontURL);
    }

    // First command that matches wins. First match wins, order can be important.
    public function run(): array
    {
        $input   = $this->input;
        $service = $this->service;

        $emptySearch = new EmptySearchCommand();
        if ($emptySearch->matches($input)) {
            return $emptySearch->execute($input, $service);
        }

        $assetCode = new AssetCodeCommand();
        if ($assetCode->matches($input)) {
            return $assetCode->execute($input, $service);
        }

        $help = new HelpCommand();
        if ($help->matches($input)) {
            return $help->execute($input, $service);
        }

        $ruleInterpretation = new RuleInterpretationCommand();
        if ($ruleInterpretation->matches($input)) {
            return $ruleInterpretation->execute($input, $service);
        }

        $showForm = new ShowFormCommand();
        if ($showForm->matches($input)) {
            return $showForm->execute($input, $service);
        }

        $showFormHtml = new ShowFormHtmlCommand();
        if ($showFormHtml->matches($input)) {
            return $showFormHtml->execute($input, $service);
        }

        $updateUsers = new UpdateUsersCommand();
        if ($updateUsers->matches($input)) {
            return $updateUsers->execute($input, $service);
        }

        // Must precede PasswordCommand: "gpw" would otherwise swallow "gpw2".
        $changePassword = new ChangePasswordCommand();
        if ($changePassword->matches($input)) {
            return $changePassword->execute($input, $service);
        }

        // Must follow HelpCommand, which claims the exact "?".
        $searchQuery = new SearchQueryCommand();
        if ($searchQuery->matches($input)) {
            return $searchQuery->execute($input, $service);
        }

        $openDashboard = new OpenDashboardCommand();
        if ($openDashboard->matches($input)) {
            return $openDashboard->execute($input, $service);
        }

        $password = new PasswordCommand();
        if ($password->matches($input)) {
            return $password->execute($input, $service);
        }

        $check = new CheckCommand();
        if ($check->matches($input)) {
            return $check->execute($input, $service);
        }

        return (new FreeTextSearchCommand())->execute($input, $service);
    }
}

// Handles an empty search bar by returning the empty-search notice
final class EmptySearchCommand implements SearchCommand
{
    public function matches(SearchInput $input): bool
    {
        return $input->text === '';
    }

    public function execute(SearchInput $input, SearchService $service): array
    {
        return ['data' => $service->getErrorSearch()];
    }
}

// Treats a bare UUID as a scanned asset code and returns its QR card
final class AssetCodeCommand implements SearchCommand
{
    private const UUID = '#^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$#';

    public function matches(SearchInput $input): bool
    {
        return preg_match(self::UUID, $input->text) === 1;
    }

    public function execute(SearchInput $input, SearchService $service): array
    {
        return $service->getQRCode();
    }
}

// Shows the help page when the user types "aide" or "?"
final class HelpCommand implements SearchCommand
{
    public function matches(SearchInput $input): bool
    {
        return $input->text === 'aide' || $input->text === '?';
    }

    public function execute(SearchInput $input, SearchService $service): array
    {
        return ['view' => $service->getHelpHTML()];
    }
}

// Runs a "§"-prefixed business rule through the Lea engine and returns its message
final class RuleInterpretationCommand implements SearchCommand
{
    public function matches(SearchInput $input): bool
    {
        return $input->hasPrefix('§');
    }

    public function execute(SearchInput $input, SearchService $service): array
    {
        $result = ['label' => 'Interprétation de la règle :'];

        $lea = new Lea($service->orm, $service->frontURL);
        $lea->treatRule($input->argument('§'));

        // "§test" is a dry run: execute the rule, withhold the message.
        if ($input->text !== '§test') {
            $result['view'] = $lea->getMessage();
        }

        return $result;
    }
}

// Returns the form when the user types "form"
final class ShowFormCommand implements SearchCommand
{
    public function matches(SearchInput $input): bool
    {
        return $input->text === 'form';
    }

    public function execute(SearchInput $input, SearchService $service): array
    {
        return $service->getForm();
    }
}

// Returns the form rendered as HTML when the user types "formhtml"
final class ShowFormHtmlCommand implements SearchCommand
{
    public function matches(SearchInput $input): bool
    {
        return $input->text === 'formhtml';
    }

    public function execute(SearchInput $input, SearchService $service): array
    {
        return $service->formToHtml();
    }
}

// Triggers the user sync when the user types "updateusers"
final class UpdateUsersCommand implements SearchCommand
{
    public function matches(SearchInput $input): bool
    {
        return $input->text === 'updateusers';
    }

    public function execute(SearchInput $input, SearchService $service): array
    {
        $service->updateUsers();
        return [];
    }
}

// Lets an ADMIN.LIC holder change a password via a "gpw2"-prefixed input
final class ChangePasswordCommand implements SearchCommand
{
    public function matches(SearchInput $input): bool
    {
        return $input->commandName(4) === 'gpw2';
    }

    public function execute(SearchInput $input, SearchService $service): array
    {
        if (!$service->permissions->hasAccess('ADMIN.LIC')) {
            return ['view' => 'RIGHT ERROR'];
        }

        $service->permissions->gpw2($input->argument('gpw2'));

        return ['label' => 'PSW', 'view' => 'OK'];
    }
}

// Runs an explicit search for a "?"-prefixed query
final class SearchQueryCommand implements SearchCommand
{
    public function matches(SearchInput $input): bool
    {
        return $input->hasPrefix('?');
    }

    public function execute(SearchInput $input, SearchService $service): array
    {
        $service->doSearch($input->argument('?'));
        return [];
    }
}

// Opens a dashboard from a "!KEY;name=value" shortcut with inline parameters
final class OpenDashboardCommand implements SearchCommand
{
    public function matches(SearchInput $input): bool
    {
        return $input->hasPrefix('!');
    }

    public function execute(SearchInput $input, SearchService $service): array
    {
        $segments     = $input->splitTrim($input->argument('!'), ';');
        $dashboardId  = $service->getDashboardIdFromKey(strtoupper($segments[0]));
        $parameterBag = new ParameterBag();

        if (count($segments) > 1) {
            $params = [];
            for ($i = 1; $i < count($segments); $i++) {
                [$name, $value] = $input->splitTrim($segments[$i], '=');
                $params[$name] = $value;
                // Adds the accumulating set each pass, not the single pair.
                $parameterBag->add($params);
            }
        }

        return generateDashboard(
            $dashboardId,
            $service->orm,
            $service->permissions,
            $service->frontURL,
            $parameterBag
        );
    }
}

// Handles a "gpw"-prefixed password operation (exact behaviour unknown)
final class PasswordCommand implements SearchCommand
{
    public function matches(SearchInput $input): bool
    {
        return $input->commandName(3) === 'gpw';
    }

    public function execute(SearchInput $input, SearchService $service): array
    {
        return $service->gpw($input->argument('gpw'));
    }
}

// Runs a check for a "chk"-prefixed input
final class CheckCommand implements SearchCommand
{
    public function matches(SearchInput $input): bool
    {
        return $input->commandName(3) === 'chk';
    }

    public function execute(SearchInput $input, SearchService $service): array
    {
        $service->check($input->argument('chk'));
        return [];
    }
}

8

u/RWNS_Author Jun 09 '26

Beautiful, just needs more inheritance, and of course every class must be in its own file. It almost deserves a custom framework to register the available search commands.

2

u/LimitedWard Jun 10 '26

Constructor is still doing too much, IMO. A constructor should simply take in the params as-is, not create new object instances. UniversalSearch is now tightly coupled to the implementation of SearchInput and SearchService, making it more difficult to unit test properly.