Корректная оценка выражения

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

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

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

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

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

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

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

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

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

Answer 2

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

Answer 3

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

Answer 4

также первый является двойным вложенным Тернарным оператором

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

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

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

Sidenote:

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

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

вместо

return X; 

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

Answer 5

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

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

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

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

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

Answer 6

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

За несколько лишних миллисекунд, вы можете переписать этот метод полезности в качестве метода расширения, как так:

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 7

Другой вариант: (! = Значение нуль)

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