PHP function to evaluate comparisons between two values, with debug logging

Question

The following method works fine but it is rather verbose:

private function performComparison($element, $operator, $value_allowed, $value_entered)
{
    switch($operator):
        case '>':
            $result = ($value_entered >  $value_allowed) ? true : false;
            break;
        case '>=':
            $result = ($value_entered >= $value_allowed) ? true : false;
            break;
        case '<':
            $result = ($value_entered <  $value_allowed) ? true : false;
            break;
        case '<=':
            $result = ($value_entered <= $value_allowed) ? true : false;
            break;
        case '=':
            $result = ($value_entered == $value_allowed) ? true : false;
            break;
    endswitch;
        $dateformat = 'n/j/Y';
        $debug = ['element' => $element, 'operator' => $operator, 'value allowed' => is_a($value_allowed, 'DateTime') ? $value_allowed->format($dateformat) : $value_allowed, 'value entered' => is_a($value_entered, 'DateTime') ? $value_entered->format($dateformat) : $value_entered, 'result' => ($result === false) ? 'false' : 'true'];
        echo '<pre>'.print_r($debug,1).'</pre>';
    return $result;
}

Note that the 3 lines of code just before the return $result; line are very helpful for debugging in the Development phase so I'd like to keep those.

In an attempt to make a more concise version of this method I came up with the following:

private function performComparison($element, $operator, $value_allowed, $value_entered)
{
    $comparisonFunctions = [
        '>' => function($value_allowed, $value_entered) { return ($value_entered >  $value_allowed) ? true : false; },
        '>='    => function($value_allowed, $value_entered) { return ($value_entered >= $value_allowed) ? true : false; },
        '<' => function($value_allowed, $value_entered) { return ($value_entered <  $value_allowed) ? true : false; },
        '<='    => function($value_allowed, $value_entered) { return ($value_entered <= $value_allowed) ? true : false; },
        '=' => function($value_allowed, $value_entered) { return ($value_entered == $value_allowed) ? true : false; }
    ];

    $result = array_key_exists($operator, $comparisonFunctions) ? $comparisonFunctions[$operator]($value_allowed, $value_entered) : null;
        $dateformat = 'n/j/Y';
        $debug = ['element' => $element, 'operator' => $operator, 'value allowed' => is_a($value_allowed, 'DateTime') ? $value_allowed->format($dateformat) : $value_allowed, 'value entered' => is_a($value_entered, 'DateTime') ? $value_entered->format($dateformat) : $value_entered, 'result' => ($result === false) ? 'false' : 'true'];
        echo '<pre>'.print_r($debug,1).'</pre>';
    return $result;
}

This version also works fine but I'm wondering if there are any ways it could be condensed further.

At the very least, ... ? true : false is always too verbose and can be omitted. — deceze
– deceze, Commented Jul 10, 2018 at 12:10
Could you explain what this function is actually for? I'm finding it hard to understand why you'd do something like performComparison($element, $operator, $value_allowed, $value_entered) instead of just doing the comparison. If the purpose of the function is to assist debugging then you should have a logComparison function that you call after making a comparison not performComparsion where it just has those 3 lines after the switch. I know if I came across a code base that made use of a performComparison function I would forget to use it in favor of making the actual comparison. — Shelby115
– Shelby115, Commented Jul 10, 2018 at 12:26

Your Common Sense · Accepted Answer · 2018-07-11 03:19:51Z

given your function's purpose is to return the calculation result, I would return the expression result right away, especially given return makes break unnecessary as well

private function performComparison($operator, $value_allowed, $value_entered)
{
    switch($operator) {
        case '>':
            return ($value_entered >  $value_allowed);
        case '>=':
            return ($value_entered >= $value_allowed);
        case '<':
            return ($value_entered <  $value_allowed);
        case '<=':
            return ($value_entered <= $value_allowed);
        case '=':
            return ($value_entered == $value_allowed);
        default:
            throw new InvalidArgumentException("Invalid operator");
    }
}

The code above is both concise and readable, it takes one no problem to gist the function's meaning and purpose.

Whereas any debugging should be done elsewhere. If you need any debugging for this function, create another one encapsulating the current function's call.

    private function debugPerformComparison($element, $operator, $value_allowed, $value_entered)
    {
        $result = $this->performComparison($operator, $value_allowed, $value_entered);
        $dateformat = 'n/j/Y';
        $debug = [
            'element' => $element, 
            'operator' => $operator, 
            'value allowed' => is_a($value_allowed, 'DateTime') ? $value_allowed->format($dateformat) : $value_allowed, 
            'value entered' => is_a($value_entered, 'DateTime') ? $value_entered->format($dateformat) : $value_entered, 
            'result' => $result,
        ];
        echo '<pre>'.var_export($debug,1).'</pre>';
        return $result;
    }

Regarding anonymous function-based version, a concise version shouldn't be less readable in the first place. Any code improvements should improve the readability, by means of removing unnecessary parts, not worsen it. And the said version is extremely hard to read.

Stack Exchange Network

PHP function to evaluate comparisons between two values, with debug logging

1 Answer 1

You must log in to answer this question.

Hot Network Questions

PHP function to evaluate comparisons between two values, with debug logging

1 Answer 1

You must log in to answer this question.

Related

Hot Network Questions