Рефакторинг если-то еще, если - еще

Question

У меня есть следующий пример кода

if(object.Time > 0 && <= 499) 
{ 
    rate = .75m 
} 
else if(object.Time >= 500 && <= 999) 
{ 
    rate = .85m 
} 
else if(object.Time >= 1000) 
{ 
    rate = 1.00m 
} 
else 
{ 
    rate = 0m; 
} 

Мой вопрос, что шаблон дизайна можно использовать, чтобы сделать это лучше?

Редактирование: просто для уточнения немного лучше, код, который вы видите здесь, является тем, что в настоящее время существует в рамках реализации шаблона стратегии. У нас есть 3 типа вычислений, 2 из которых имеют 3 разных «тарифа», которые можно использовать в зависимости от времени, которое вы видите ниже. Я думал о создании реализации стратегии для каждой скорости, но тогда я бы двигал логикой для определения стратегии использования и делал это беспорядок.

Спасибо!

Answer 1

Если вы действительно ищете шаблон дизайна, я бы выбрал шаблон «Цепь ответственности».

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

Так у вас есть это абстрактный класс, который каждое звено унаследует:

public abstract class Link 
{ 
    private Link nextLink; 

    public void SetSuccessor(Link next) 
    { 
     nextLink = next; 
    } 

    public virtual decimal Execute(int time) 
    { 
     if (nextLink != null) 
     { 
      return nextLink.Execute(time); 
     } 
     return 0; 
    } 
} 

И затем вы создаете каждый связи с вашими правилами:

public class FirstLink : Link 
{ 
    public override decimal Execute(int time) 
    { 
     if (time > 0 && time <= 499) 
     { 
      return .75m; 
     } 

     return base.Execute(time); 
    } 
} 

public class SecondLink : Link 
{ 
    public override decimal Execute(int time) 
    { 
     if (time > 500 && time <= 999) 
     { 
      return .85m; 
     } 

     return base.Execute(time); 
    } 
} 

public class ThirdLink : Link 
{ 
    public override decimal Execute(int time) 
    { 
     if (time >= 1000) 
     { 
      return 1.00m; 
     } 

     return base.Execute(time); 
    } 
} 

Наконец, чтобы использовать его, просто установите весь преемник и назовите его:

Link chain = new FirstLink(); 
Link secondLink = new SecondLink(); 
Link thirdLink = new ThirdLink(); 


chain.SetSuccessor(secondLink); 
secondLink.SetSuccessor(thirdLink); 

и все, что вам нужно сделать, это вызов цепи с помощью одного чистого вызова:

Answer 2

Вы можете выбрать формат, а не шаблон проектирования в состоянии if-else;

Как правило, если у вас есть lot of conditions, то я предпочитаю if, чем много вложенных if-else, вы можете выбрать что-то вроде этого;

if(condition1){ 
    return x; // or some operation 
} 

if(condition 2){ 
    return y; // or some operation 
} 

return default; // if none of the case is satisfied. 

Answer 3

Я не думаю, что это анти-шаблон. Проблема и для кодовых метрик - также oki. Если не вложен и не очень сложный !. Но вы могли бы сделать лучше, например, с помощью коммутатора или сделать собственный класс, содержащий свойства IsAgeBiggerThanMax() и т.д.

---

обновление Switch:

 var range = (time - 1)/499; 
     switch (range) 
     { 
      case 0: // 1..499 
       rate = 0.75; 
       break; 
      case 1: // 500.. 999 
       rate = 0.85; 
       break; 
      default: 
       rate = 0; 
       if (time == 1000) 
       { 
        rate = 1.0; 
       } 
       break; 
     } 

Тестирование философии вопрос мы не знать, что это за функция и что делает. возможно, он может быть протестирован на 100% снаружи!

Answer 4

Существует не столь известная картина под названием «правила Pattern»

Идея заключается в том, что все извлекается в объект и пусть обрабатывать свою собственную работу. У вас будет каждого класса для каждого правила, которое вы определили, который является вашим заявлением о состоянии, например. (object.Time> 0 & & < = 499)

public class RuleNumberOne : IRules 
{ 
    public decimal Execute(Oobject date) 
    { 
     if(date.Time > 0 && date.Something <= 499) 
     return .75m; 
     return 0; 
    } 
} 

public class RuleNumberTwo : IRules 
{ 
    public decimal Execute(Oobject date) 
    { 
     if(date.Time >= 500 && date.Something <= 999) 
      return .85m; 
     return 0; 
    } 
} 

public interface IRules 
{ 
    decimal Execute(Oobject date); 
} 

Таким образом, на классе, который имел обыкновение выглядеть

if(object.Time > 0 && <= 499) 
{ 
    rate = .75m 
} 
else if(object.Time >= 500 && <= 999) 
{ 
    rate = .85m 
} 
else if(object.Time >= 1000) 
{ 
    rate = 1.00m 
} 
else 
{ 
    rate = 0m; 
} 

теперь будет,

private List<IRules>_rules = new List<IRules>(); 
public SomeConstructor() 
{ 
    _rules.Add(new RuleNumberOne()); 
    _rules.Add(new RuleNumberTwo()); 
} 

public void DoSomething() 
{ 

    Oobject date = new Oobject(); 

    foreach(var rule in this._rules) 
    { 
     Decimal rate = rule.Execute(date); 
    } 
} 

Идея здесь в том, что как только вы попадете в гнездо, если condi было бы труднее прочитать заявления о состоянии, и разработчику было бы сложно внести какие-либо изменения. Таким образом, он отделяет логику каждого отдельного правила и его эффект в свой класс, который следует за Шаблоном единой ответственности.

Некоторые соображения 1.) только для чтения 2.) Явная порядок 3.) Зависимости 4.) Приоритет 5.) Постоянство

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

Вы можете настроить его, если хотите, не хотите, чтобы он возвращал десятичную или что-то, но идея здесь.

Answer 5

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

Class 
    FromTime 
    ToTime 
    Value 

values.Add(New Class(0, 499, .75)); 
values.Add(New Class(500, 999, .85)); 
values.Add(New Class(1000, 9999, 1)); 

Затем цикл каждый элементы коллекции

if(object.Time >= curItem.FromTime && object.Time <= curItem.ToTime) 
    rate = curItem.Value; 

Вы можете всегда иметь NULLABLE значения или задать -1, как бесконечна.

values.Add(New Class(-1, 0, 0)); 
values.Add(New Class(0, 499, .75)); 
values.Add(New Class(500, 999, .85)); 
values.Add(New Class(1000, -1, 1)); 

if((object.Time >= curItem.FromTime || curItem.FromTime == -1) && (object.Time <= curItem.ToTime || curItem.ToTime == -1)) 
    rate = curItem.Value; 

Answer 6

Просто сделайте одно сравнение в каждом случае, и идти сверху-вниз со значениями:

if (Object.Time >= 1000) 
    rate = 1.0; 
else 
    if (Object.Time >= 500) 
     rate = 0.85; 
    else 
     if (Object.Time > 0) 
      rate = 0.75; 
     else 
      rate = 0; 

Answer 7

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

if (obj.Time <= 0) { 
    rate = 0.00m; 
} 

// At this point, obj.Time already must be >= 0, because the test 
// to see if it was <= 0 returned false. 
else if (obj.Time < 500) { 
    rate = 0.75m; 
} 

// And at this point, obj.Time already must be >= 500. 
else if (obj.Time < 1000) { 
    rate = 0.85m; 
} 

else { 
    rate = 1.00m; 
} 

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

Answer 8

Мне очень нравится решение Лев Лоренцо Луис. Но вместо того, чтобы возвращать Rate, я бы позволил Правилу что-то с этим сделать. Это будет уважать S. от S.O.L.I.D. Principles и Law Of Demeter.

Кроме того, когда класс «запрашивает» значение, которое содержится в другом классе, вы можете идентифицировать его как smell, называемый data class. Вы должны стараться избегать этого.

Это, как говорится: Я хотел бы сделать две вещи, чтобы полировать решение Лео Лоренцо:

1. Вызовите правой Rule без for loop.
2. Выполнение поведения, запрошенного внутри его связанного класса Rule.

Для этого нам необходимо сопоставить классы rule с их временным диапазоном, поэтому к ним можно получить доступ напрямую, вместо того чтобы проходить через цикл. Вам нужно реализовать свой собственный Map объект (или список объектов или коллекции), перегрузки [] operator и это add функция

так что вы можете добавить свои правила в вашей карте, как это, например:

ranges.Add(0,500).AddRule(rule1); 
ranges.Add(500,1000).AddRule(rule2); 
etc.. 

Вы можете видеть выше, есть объект Range, который может иметь связанный с ним объект Rule. Таким образом, вы можете в итоге добавить несколько правил для того же Range.

Тогда вы называете это так:

ranges[Object.time].Execute(Object); 

Answer 9

Использование карты:

var map = new[] 
{ 
    new { Rule = (Func<Oobject, bool>) (x => x.Time > 0 && x.Something <= 499), 
      Value = .75m }, 
    new { Rule = (Func<Oobject, bool>) (x => x.Time >= 500 && x.Something <= 999), 
      Value = .85m }, 
    new { Rule = (Func<Oobject, bool>) (x => true), 
      Value = 0m } 
}; 

var date = new Oobject { Time = 1, Something = 1 }; 
var rate = map.First(x => x.Rule(date)).Value; 

Assert.That(rate, Is.EqualTo(.75m)); 

---

Мне нравится идея Rules Pattern ответ @ LLL, но у него есть недостаток.

Рассмотрим следующий тест (NUnit):

[Test] 
public void TestRulesUsingList() 
    var rules = new IRules[]{ new RuleNumberOne(), new RuleNumberTwo() }; 

    var date = new Oobject { Time = 1, Something = 1 }; 
    var rate = 0m; 

    foreach(var rule in rules) 
     rate = rule.Execute(date); 

    Assert.That(rate, Is.EqualTo(.75m)); 
} 

тест не пройден. Хотя было вызвано RuleNumberOne и возвращено ненулевое значение, впоследствии было вызвано RuleNumberTwo и возвращено ноль, чтобы перезаписать правильное значение.

Чтобы воспроизвести логику if..else..else, она должна иметь возможность короткого замыкания.

Вот быстрое решение: изменить Execute метод Интерфейс для возврата bool, чтобы указать, следует ли исключить должны стрелять и добавить Value свойство, чтобы получить decimal ценность правила. Также добавьте правило defulat, которое alwasys оценивает true и возвращает ноль. Затем измените реализацию (тест), чтобы получить значение первого правила для оценки истины:

[Test] 
public void TestRulesUsingList2() 
{ 
    var rules = new IRules[]{ new RuleNumberOne(), new RuleNumberTwo(), 
     new DefaultRule() }; 

    var date = new Oobject { Time = 1, Something = 1 }; 
    var rate = rules.First(x => x.Execute(date)).Value; 

    Assert.That(rate, Is.EqualTo(.75m)); 
} 

public class Oobject 
{ 
    public int Time { get; set; } 
    public int Something { get; set; } 
} 

public interface IRules 
{ 
    bool Execute(Oobject date); 
    decimal Value { get; } 
} 

public class RuleNumberOne : IRules 
{ 
    public bool Execute(Oobject date) 
    { 
     return date.Time > 0 && date.Something <= 499; 
    } 

    public decimal Value 
    { 
     get { return .75m; } 
    } 
} 

public class RuleNumberTwo : IRules 
{ 
    public bool Execute(Oobject date) 
    { 
     return date.Time >= 500 && date.Something <= 999; 
    } 

    public decimal Value 
    { 
     get { return .85m; } 
    } 
} 

public class DefaultRule : IRules 
{ 
    public bool Execute(Oobject date) 
    { 
     return true; 
    } 

    public decimal Value 
    { 
     get { return 0; } 
    } 
}