Правильная оценка выражения

Question

Я наткнулся на следующее выражение в чужом коде. Я думаю, что это ужасный код по ряду причин (не в последнюю очередь потому, что он не учитывает bool.TrueString и bool.FalseString), но мне интересно, как компилятор оценит его.

private bool GetBoolValue(string value)
{
    return value != null ? value.ToUpper() == "ON" ? true : false : false;
}

Редактировать Кстати, не оцениваются ли выражения изнутри наружу? В этом случае, какой смысл проверять значение! = Null после вызова value.ToUpper (), который выдаст исключение нулевой ссылки?

Я думаю, что следующее является верной (намеренно) многословной версией (я бы никогда не оставил ее так: D):

if (value != null)
{
    if (value.ToUpper() == "ON") 
    {
        return true;
    }
    else        // this else is actually pointless
    {
        return false;
    }
}
else
{
    return false;
}

Что может быть сокращено до:

return value != null && value.ToUpper == "ON";

Это правильное переписывание выражения?

Answer 1

Похоже, что метод имеет отступ для обработки значения, полученного из элемента checkbox HTML. Если для флажка не указано значение, по умолчанию используется значение "on". Если этот флажок не установлен, то в данных формы его значение вообще отсутствует, поэтому чтение ключа из Request.Form дает нулевую ссылку.

В этом контексте метод правильный, хотя и довольно ужасный из-за использования антишаблона if-condition-then-true-else-false. Также ему нужно было дать имя, более подходящее для его конкретного использования, например GetCheckboxValue.

Ваше переписывание метода является правильным и правильным. Поскольку значение не зависит от культуры, преобразование значения в верхний регистр не должно использовать текущую культуру. Так что переписать даже лучше, чем тот, который вы предложили, будет:

return value != null && value.ToUpperInvariant == "ON";

(Методы, не зависящие от культуры, также немного быстрее, чем методы, использующие конкретную культуру, поэтому нет причин не использовать их.)

Кстати, это не выражения оценивается изнутри наружу?

Если бы это были вызовы методов, чтобы все выражения были фактически вычислены, они бы это сделали, поскольку внутренний вызов должен быть выполнен для оценки параметров внешнего вызова.

Однако второй и третий операнды условного выражения вычисляются только в том случае, если они используются, поэтому выражения оцениваются извне и вовнутрь. Внешнее условие оценивается первым, чтобы решить, какой из операндов он будет оценивать.

Answer 2

Вы правы как в своем переписывании, так и в утверждении, что эта попытка краткости плоха, потому что приводит к путанице.

Answer 3

ну, первый из них является двойным тройным оператором

return (value != null) ? [[[value.ToUpper() == "ON" ? true : false]]] : false;

Бит в [[[]]] является первым результатом троичного выражения, которое вычисляется когда первое условие истинно, значит, вы читаете / утверждаете, что оно правильное

но он ужасен как ад и очень нечитаем / не поддерживается в текущем состоянии. Я определенно изменил бы это на ваше последнее предложение

Sidenote:

Люди, которые делают

if(X == true) 
   return true;
else
   return false;

вместо

 return X;

должен быть вынут и застрелен; -)

Answer 4

Вы ищете скорость, удобочитаемость и организацию? Скорость выполнения, ваш сокращенный пример, вероятно, лучший путь.

В течение нескольких дополнительных миллисекунд вы можете переписать этот служебный метод как метод расширения следующим образом:

public static bool ToBoolean(this string value)
{

    // Exit now if no value is set
    if (string.IsNullOrEmpty(value)) return false;

    switch (value.ToUpperInvariant())
    {
        case "ON":
        case "TRUE":
            return true;
    }

    return false;

}

... и тогда вы будете использовать его следующим образом:

public static void TestMethod()
{
    bool s = "Test".ToBoolean();
}

EDIT: На самом деле, я ошибаюсь ... быстрый тест производительности показывает, что метод расширения FASTER , чем встроенный метод. Источник моего теста ниже, а также вывод на моем ПК.

[Test]
public void Perf()
{

    var testValues = new string[] {"true", "On", "test", "FaLsE", "Bogus", ""};
    var rdm = new Random();
    int RunCount = 100000;
    bool b;
    string s;

    Stopwatch sw = Stopwatch.StartNew();
    for (var i=0; i<RunCount; i++)
    {
        s = testValues[rdm.Next(0, testValues.Length - 1)];
        b = s.ToBoolean();
    }
    Console.Out.WriteLine("Method 1: {0}ms", sw.ElapsedMilliseconds);

    sw = Stopwatch.StartNew();
    for (var i = 0; i < RunCount; i++)
    {
        s = testValues[rdm.Next(0, testValues.Length - 1)];
        b = s != null ? s.ToUpperInvariant() == "ON" ? true : s.ToUpperInvariant() == "TRUE" ? true : false : false;
    }
    Console.Out.WriteLine("Method 2: {0}ms", sw.ElapsedMilliseconds);


}

Выход:

Method 1: 21ms
Method 2: 30ms

Answer 5

Другая альтернатива:

return string.Equals( value, "ON", StringComparison.OrdinalIgnoreCase );

Answer 6

Я также ненавижу такие конструкции, как:

if (value.ToUpper() == "ON") 
{
    return true;
}
else        // this else is actually pointless
{
    return false;
}

как вы заметили, это длинный и запутанный (не сказать глупый) способ записи:

return value.ToUpper() == "ON";

Ваше предложение хорошее, короткое и правильное.

Answer 7

Я читаю оригинальное выражение так же, как вы. Поэтому я думаю, что ваше укороченное выражение является правильным. Если значение равно нулю, оно никогда не перейдет ко второму условию, поэтому оно выглядит безопасным для меня.