Оптимизация функции сравнения

[nigirma]

Есть в самописном валидаторе функция сравнения, которую очень хочется оптимизировать.

/**
 * Сравнение значения со значением другого
 * 
 * data     string|number Сравниваемое значение
 * value    string|number Значение для сравнения
 * operator string        Оператор, используемый при сравнении
 * message  string        Текст сообщения при ошибке
 */
function compare($data, $comparison, $operator = '==', $message = '') {
    if ($operator == '==') {
        if ($data == $comparison) {
            $compareError = true;
        }
    } else if (($operator == '!=') || ($operator == '<>')) {
        if ($data != $comparison) {
            $compareError = true;
        }
    } else if ($operator == '<=') {
        if ($data <= $comparison) {
            $compareError = true;
        }
    } else if ($operator == '>=') {
        if ($data >= $comparison) {
            $compareError = true;
        }
    } else if ($operator == '<') {
        if ($data < $comparison) {
            $compareError = true;
        }
    } else if ($operator == '>') {
        if ($data > $comparison) {
            $compareError = true;
        }
    }
    return (isset($compareError)) ? true : $message;
}

// Проверка функции
echo compare(5, 4, '==', 'Числа не равны') . '<br>';
echo compare(5, 5, '==', 'Числа не равны');

1) Не хотелось бы использовать case - будет больше кода
2) объединение

    if ($operator == '==') {
        if ($data == $comparison) {
            $compareError = true;
        }
    }

в

    if (($operator == '==') && ($data == $comparison)) {
        $compareError = true;
    }

будет только хуже, т.к. если $data != $comparison то проверка по if'ам вниз пойдет дальше, что уже не даст результата совпадения $operator.

Какие будут варианты оптимизации?? Или все так хорошо?

 

[С.]

Во-первых, откройте для себя elseif.

Во-вторых, вместо

        if ($data == $comparison) {
            $compareError = true;
        }

всяко лучше

$compareError= ($data == $comparison)

А в-третьих, если функция предусматривает только эти операторы, то оптимальнее будет:

function compare($comparison, $message = '')
{...}

echo compare($х==4, 'Числа не равны');

 

[~WR~]

С case нагляднее всё же.

function compare($a, $b, $op = '==', $error_msg = '') 
{
	$ok = false;
	
	switch($op)
	{
		case '==':
			$ok = $a == $b;
		break;
			
		case '!=':
		case '<>':
			$ok = $a != $b;
		break;
		
		case '<=':
			$ok = $a <= $b;
		break;
		
		case '>=':
			$ok = $a >= $b;
		break;
		
		case '<':
			$ok = $a < $b;
		break;
		
		case '>':
			$ok = $a > $b;
		break;
		
		default:
			//invalid op
		break;
	}
	
	return $ok ?: $error_msg;
}

С., при сравнении скобки не нужны. Оператор присваивания имеет сильно меньший приоритет, чем сравнение.
http://php.net/manual/en/language.operators.precedence.php

 

[Mols]

Так получше будет

function compare($a, $b, $op = '==', $error_msg = '') 
{
    switch($op)
    {
        case '==':
            return  $a == $b;
            
        case '!=':
        case '<>':
            return  $a != $b;
        
        case '<=':
            return  $a <= $b;
        
        case '>=':
            return  $a >= $b;
        
        case '<':
            return  $a < $b;
 
        case '>':
            return $a > $b;      
    }
    
    return  $error_msg;
}

Но тут конечно подход не правильный.
При ошибке - должно быть исключение.
Да и вообще тут речь похоже не о оптимизации а именно о читаемости кода

 

[~WR~]

Угу, здорово) Без лишних break'ов.

 

[Mols]

да самое главное что вот этого нет

return $ok ?: $error_msg;

$ok - вполне может иметь валидное значение false
А в описанном выше варианте из функции никогда не может быть возвращено false.
Только true или сообщение об ошибке.

 

[~WR~]

А, я понял, почему сам переменную использовал.
У автора так и есть. Либо true, либо сообщение об ошибке. Посмотрите первый пост.

 

[radioheaded]

Я бы сделал отдельный класс с методами-операциями (eq, lt, gt, ...), на входе в этой функции при помощи ассоциативной карты преобразовывал бы оператор в имя метода (никаких условных операторов), вызывал бы нужный метод, передавая в него операнды. Такое решение потом очень легко поддерживать и расширять.

 

[Sad Spirit]

У меня в "самописном валидаторе" делается "оптимизация" через create_function():

class HTML_QuickForm2_Rule_Compare extends HTML_QuickForm2_Rule
{
// ...
    protected function validateOwner()
    {
        $value  = $this->owner->getValue();
        $config = $this->getConfig();
        if (!in_array($config['operator'], array('===', '!=='))) {
            $compareFn = create_function(
                '$a, $b', 'return floatval($a) ' . $config['operator'] . ' floatval($b);'
            );
        } else {
            $compareFn = create_function(
                '$a, $b', 'return strval($a) ' . $config['operator'] . ' strval($b);'
            );
        }
        return $compareFn($value, $config['operand'] instanceof HTML_QuickForm2_Node
                                  ? $config['operand']->getValue(): $config['operand']);
    }
// ...
}

На мой взгляд, это с децел проще читать.

 

[nigirma]

Переубедился в использовании case.
Код стал более читабельным и короче.
Спасибо за ответы.

ЗЫ: как заметил Mols, нужна была не оптимизация кода, а улучшение читабельности.