Сокращение вложенных IFs

Question

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

if (mode == 1) 
{ 
    if (distance <= 4000) 
    { 
     modeValue = "1F"; 
    } 

    else if (distance > 4000 && distance <= 8000) 
    { 
     modeValue = "2F"; 
    } 

    else if (distance > 8000 && distance <= 12000) 
    { 
     modeValue = "3F"; 
    } 

    else if (distance > 12000) 
    { 
     modeValue = "F 0-5"; 
    } 
} 

else if (mode == 2) 
{ 
    if (distance <= 500) 
    { 
     modeValue = ""; 
    } 

    else if (distance > 500 && distance <= 4000) 
    { 
     modeValue = "2F"; 
    } 

    else if (distance > 4000 && distance <= 8000) 
    { 
     modeValue = "3F"; 
    } 

    else if (distance > 8000 && distance <= 12000) 
    { 
     modeValue = "4F"; 
    } 

    else if (distance > 12000) 
    { 
     modeValue = "F 0-5"; 
    } 
} 

Любые предложения?

Answer 1

Прежде всего, вы должны заменить операторы if-то иметь дело с mode с переключателем:

switch (mode) { 
case 1: 
    //... 
    break; 
case 2: 
    //... 
    break; 
default: break; 
} 

Затем вы можете заменить внутренние if-то еще цепи с коммутаторами, помещая их в методе (в они называются «процедурами» в C#?):

string Foo(int mode, int distance) { 
    switch (mode) 
    { // http://stackoverflow.com/users/469563/gendoikari Thanks for pointing out redundant comparisons 
    case 1: 
     if (distance <= 4000) 
     { return "1F"; } 
     else if (distance <= 8000) 
     { return "2F"; } 
     else if (distance <= 12000) 
     { return "3F"; } 
     else 
     { return "F 0-5"; } 
    case 2: 
     if (distance <= 500) 
     { return ""; } 
     else if (distance <= 4000) 
     { return "2F"; } 
     else if (distance <= 8000) 
     { return "3F"; } 
     else if (distance <= 12000) 
     { return "4F"; } 
     else 
     { return "F 0-5"; } 
    default: break; 
    } 

О, смотри, что-то удивительное произошло случайно! Теперь вы можете просто позвонить Foo всякий раз, когда вы хотите изменить modeValue!

modeValue = Foo(mode, distance); 

Answer 2

Пара вещей:

Во-первых, у вас есть избыточные проверки

else if (distance > 500 && distance <= 4000)

может быть просто else if (distance <= 4000) вместо этого, из-за «еще» часть .. вы уже проверить, что расстояние > 500, если вы находитесь в этом.

Во-вторых, это действительно зависит от контекста, связанного с этим, но это может быть хорошим местом для использования нескольких классов. Я собираюсь предположить, что все это внутри метода CalculateModeValue, и этот метод находится внутри класса под названием Calculator. Я также предполагаю, что «Mode == 1» означает «LongDistance», а «Mode == 2» означает «ShortDistance».

В этом случае у меня будет абстрактный класс, называемый калькулятор. Затем я подклассифицировал бы это двумя отдельными классами: «LongDistanceCalculator» и «ShortDistanceCalculator». CalculateModeValue будет абстрактным методом в калькуляторе и реализован отдельно в каждом подклассе. Таким образом, вам не нужно будет проверять «If Mode == 1»; реализация каждого класса будет обрабатывать правильную логику для этого режима.

Но снова это делает предположения об неизвестных; это действительно зависит от контекста вокруг этих операторов if.

Answer 3

Нет ничего плохого в использовании вложенных, если они есть у вас там. Но если вы действительно не хотите смотреть на них; Может быть, просто введите логику в метод, который принимает переменную режима, а затем переходит к применению логики к значению режима?

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

Answer 4

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

string Func1(int mode, double distance) 
{ 
    if(mode == 1) 
    { 
     if(distance <= 4000) 
      return "1F"; 
     else if(distance <= 8000) 
      return "2F"; 
     else if(distance <= 12000) 
      return "3F"; 
     else 
      return "F 0-5"; 
    } 
    else 
    { 
     if(distance <= 500) 
      return ""; 
     else if(distance <= 4000) 
      return "2F"; 
     else if(distance <= 8000) 
      return "3F"; 
     else if(distance <= 12000) 
      return "4F"; 
     else 
      return "F 0-5"; 
    } 
} 

Если вам не нравится, что есть тонна способов сделать это.Как насчет:

class Mode 
{ 
    public double Distance; 
    public string Name; 
    public Mode(double d, string n) { this.Distance = d; this.Name = n; } 
} 

string Func(int mode, double distance) 
{ 
    Mode[] modes; 
    if(mode == 1) 
     modes = new Mode[] { new Mode(4000, "1F"), 
          new Mode(8000, "2F"), 
          new Mode(12000, "3F"), 
          new Mode(double.MaxValue, "F 0-5") }; 
    else 
     modes = new Mode[] { new Mode(500, ""), 
          new Mode(4000, "2F"), 
          new Mode(8000, "3F"), 
          new Mode(12000, "4F"), 
          new Mode(double.MaxValue, "F 0-5") }; 

    for(int pos = 0; ; pos++) 
     if(distance <= modes[pos].Distance) 
      return modes[pos].Name; 
} 

Очевидно, что вы можете повторно использовать эти массивы. Или вы можете иметь в каждом режиме Predicate<T> или что-то подобное, определяющее условие для выбора этого режима. И я думаю, что класс Mode может просто быть KeyValuePair<TKey,TValue>, чтобы сократить его еще немного.