Избегать слишком много if else, иначе if if для нескольких (9) условий

Question

У меня есть 9 условий для моего оператора if else, я думал, есть ли другое альтернативное решение, чтобы сделать код чистым и коротким.Все 9 условий выполняют разные вычисления для asv.net mvc view.Я включил код и изображение, которое показывает вид некоторых условий, надеюсь, что это имеет смысл, я искал лучшее и надежное решение для этого сценария. Спасибо*

Answer 1

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

Answer 2

Сначала перенесите извлечение значения формы в начало метода, например:

int? helmets = formvalues["helmets"] == null ?
  null : Convert.ToInt32(formvalues["helmets"];
int? garages = formvalues["garages"] == null ?
  null : Convert.ToInt32(formvalues["garages"];

Тогда вы, вероятно, очень легко сможете установить свойства без каких-либо ifs / else if, например:

ViewBag.hideHelmet = helmets;
// or
someOtherProperty = helmets == null ? ... : ...

Обновление (относительно вашего комментария / вопроса):

• Оператор ?: в выражении x = condition ? value1 : value2 называется условный оператор .Возвращает value1, если condition - истина, и value2 в противном случае.
• int? - это обнуляемое целое число , которое может иметь целочисленное значение или быть нулевым.1022 *

Answer 3

Почему бы вам не сосредоточиться на целях назначения вместо условий?

Условие кажется довольно простым и повторяющимся.Один пример:

ViewBag.hideHelmet = formvalues["helmets"] == ""? 1 : 0;

Это не требует никакой логики ветвления.

Answer 4

Несколько замечательных предложений.Мне нравится решать эти ситуации следующим образом.Я переработал ваш код, чтобы полностью удалить условные выражения.Вместо этого я инкапсулировал код для каждого условия в метод и создал хеш-таблицу с ключом типа int и значением типа Action.Я получил уникальное значение для каждой комбинации шлем / гараж, которое затем используется в качестве ключа для соответствующего метода.

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

using System;
using System.Collections;
using System.Collections.Generic;
using System.IO;

namespace StackOverflow
{
    public enum HelmetState { Null = 1, Empty = 2, HasValue = 3 }
    public enum GarageState { Null = 4, Empty = 5, HasValue = 6 }

    public class ReducingConditionals
    {
        private TrackEventCost trackEventCost = new TrackEventCost();
        private GetHelmets getHelmets = new GetHelmets();
        private GetGarages getGarages = new GetGarages();
        private IDictionary<int, Action> ExecuteCondition;

        //the formvalues collection
        private Hashtable _formvalues;
        public Hashtable formvalues{ get { return _formvalues ?? (_formvalues = new Hashtable()); } }

        //set up dictionary of Actions
        public ReducingConditionals()
        {
            ExecuteCondition = new Dictionary<int, Action>();
            ExecuteCondition[Key(HelmetState.Null, GarageState.Null)] = HelmetsNullGaragesNull;
            ExecuteCondition[Key(HelmetState.Null, GarageState.Empty)] = HelmetsNullGaragesEmpty;
            ExecuteCondition[Key(HelmetState.Null, GarageState.HasValue)] = HelmetsNullGaragesHasValue;
            ExecuteCondition[Key(HelmetState.Empty, GarageState.Null)] = HelmetsEmptyGaragesNull;
            ExecuteCondition[Key(HelmetState.Empty, GarageState.Empty)] = HelmetsEmptyGaragesEmpty;
            ExecuteCondition[Key(HelmetState.Empty, GarageState.Empty)] = HelmetsEmptyGaragesHasValue;
            ExecuteCondition[Key(HelmetState.HasValue, GarageState.Empty)] = HelmetsHasValueGaragesNull;
            ExecuteCondition[Key(HelmetState.HasValue, GarageState.Null)] = HelmetsHasValueGaragesEmpty;
            ExecuteCondition[Key(HelmetState.HasValue, GarageState.HasValue)] = AnyOtherCondition;
        }

        //gets a unique value for each HelmetState/GarageState combination to be used as a key to the dictionary
        private int Key(HelmetState helmetState, GarageState garageState)
        {
            return (int)helmetState + (int)garageState;
        }

        //Execute the appropriate method - n.b. no if statements in sight!
        public void DealWithConditions()
        {
            HelmetState helmetState = GetHelmetState(formvalues["helmets"]);
            GarageState garageState = GetGarageState(formvalues["garages"]);
            ExecuteCondition[Key(helmetState, garageState)]();
        }

        //assign helmet state enum
        private HelmetState GetHelmetState(object helmetValue)
        {
            if (helmetValue == null) return HelmetState.Null;
            if (helmetValue.ToString() == "") return HelmetState.Empty;
            if (Convert.ToInt32(helmetValue) > 0) return HelmetState.HasValue;
            throw new InvalidDataException("Unexpected parameter value");
        }

        //assign garage state enum
        private GarageState GetGarageState(object garageValue)
        {
            if (garageValue == null) return GarageState.Null;
            if (garageValue.ToString() == "") return GarageState.Empty;
            if (Convert.ToInt32(garageValue) > 0) return GarageState.HasValue;
            throw new InvalidDataException("Unexpected parameter value");
        }

        #region encapsulate conditions in methods
        private void AnyOtherCondition()
        {
            ViewBag.hideHelmet = 0;//for jquery , This value is passed to jquery script which then decides which field to hide/show
            ViewBag.hideGarages = 0;//for jquery , This value is passed to jquery script which then decides which field to hide/show
            ViewBag.SelectedHelmets = Convert.ToInt32(formvalues["helmets"]);
            ViewBag.SelectedGarages = Convert.ToInt32(formvalues["garages"]);
            ViewBag.TotalHelmets = Convert.ToInt32(formvalues["helmets"]) * getHelmets.UnitCost;
            ViewBag.TotalGarages = Convert.ToInt32(formvalues["garages"]) * getGarages.UnitCost;
            ViewBag.TotalAmount = ViewBag.TotalHelmets + ViewBag.TotalGarages + trackEventCost.UnitCost;
        }

        private void HelmetsHasValueGaragesNull()
        {
            ViewBag.hideHelmet = 1; //for jquery , This value is passed to jquery script which then decides which field to hide/show
            ViewBag.hideGarages = 0; //for jquery , This value is passed to jquery script which then decides which field to hide/show
            ViewBag.TotalHelmets = Convert.ToInt32(formvalues["helmets"]) * getHelmets.UnitCost;
            ViewBag.TotalAmount = ViewBag.TotalHelmets + trackEventCost.UnitCost;
        }

        private void HelmetsNullGaragesHasValue()
        {
            ViewBag.hideHelmet = 0; //for jquery , This value is passed to jquery script which then decides which field to hide/show
            ViewBag.hideGarages = 1; //for jquery , This value is passed to jquery script which then decides which field to hide/show
            ViewBag.SelectedGarages = Convert.ToInt32(formvalues["garages"]);
            ViewBag.TotalGarages = Convert.ToInt32(formvalues["garages"]) * getGarages.UnitCost;
            ViewBag.TotalAmount = ViewBag.TotalGarages + trackEventCost.UnitCost;
        }

        private void HelmetsNullGaragesNull()
        {
            ViewBag.SelectedHelmets = 1;//for jquery, This value is passed to jquery script which then decides which field to hide/show
            ViewBag.SelectedGarages = 1;//for jquery, This value is passed to jquery script which then decides which field to hide/show
            ViewBag.TotalHelmets = 1;
            ViewBag.TotalGarages = 1;
            ViewBag.hideHelmet = 1;
            ViewBag.hideGarages = 1;
            ViewBag.TotalAmount = trackEventCost.UnitCost;
        }

        private void HelmetsEmptyGaragesNull()
        {
            ViewBag.hideHelmet = 1;//for jquery, This value is passed to jquery script which then decides which field to hide/show
            ViewBag.hideGarages = 1;//for jquery, This value is passed to jquery script which then decides which field to hide/show
            ViewBag.SelectedHelmets = 1;
            ViewBag.TotalAmount = trackEventCost.UnitCost;
        }

        private void HelmetsNullGaragesEmpty()
        {
            ViewBag.hideHelmet = 1;//for jquery, This value is passed to jquery script which then decides which field to hide/show
            ViewBag.hideGarages = 1;//for jquery, This value is passed to jquery script which then decides which field to hide/show
            ViewBag.TotalAmount = trackEventCost.UnitCost;
        }

        private void HelmetsHasValueGaragesEmpty()
        {
            ViewBag.hideHelmet = 1;//for jquery, This value is passed to jquery script which then decides which field to hide/show
            ViewBag.hideGarages = 0;//for jquery, This value is passed to jquery script which then decides which field to hide/show
            ViewBag.SelectedGarages = 1;
            ViewBag.SelectedHelmets = Convert.ToInt32(formvalues["helmets"]);
            ViewBag.TotalGarages = 1;
            ViewBag.TotalHelmets = Convert.ToInt32(formvalues["helmets"]) * getGarages.UnitCost;
            ViewBag.TotalAmount = ViewBag.TotalHelmets + trackEventCost.UnitCost;
        }

        private void HelmetsEmptyGaragesHasValue()
        {
            ViewBag.hideHelmet = 0;//for jquery, This value is passed to jquery script which then decides which field to hide/show
            ViewBag.hideGarages = 1;//for jquery, This value is passed to jquery script which then decides which field to hide/show
            ViewBag.SelectedHelmets = 1;
            ViewBag.SelectedGarages = Convert.ToInt32(formvalues["garages"]);
            ViewBag.TotalHelmets = 1;
            ViewBag.TotalGarages = Convert.ToInt32(formvalues["garages"]) * getGarages.UnitCost;
            ViewBag.TotalAmount = ViewBag.TotalGarages + trackEventCost.UnitCost;
        }

        private void HelmetsEmptyGaragesEmpty()
        {
            ViewBag.hideHelmet = 1;//for jquery, This value is passed to jquery script which then decides which field to hide/show
            ViewBag.hideGarages = 1;//for jquery, This value is passed to jquery script which then decides which field to hide/show
            ViewBag.SelectedHelmets = 1;//
            ViewBag.SelectedGarages = 1;//
            ViewBag.TotalHelmets = 1;
            ViewBag.TotalGarages = 1;
            ViewBag.TotalAmount = 1;
            ViewBag.TotalAmount = trackEventCost.UnitCost;
        }
        #endregion
    }

    #region dummy class class implementations
    public class ViewBag
    {
        public static int TotalAmount;
        public static int hideHelmet { get; set; }
        public static int hideGarages { get; set; }
        public static int SelectedHelmets { get; set; }
        public static int SelectedGarages { get; set; }
        public static int TotalGarages { get; set; }
        public static int TotalHelmets { get; set; }
    }
    internal class GetGarages { public int UnitCost; }
    internal class GetHelmets { public int UnitCost; }
    internal class TrackEventCost{ public int UnitCost;}
    #endregion
}

Answer 5

Как насчет создания класса условий

public class Condition
{
    public Func<bool> Match { get; set; }
    public Action Execute { get; set; }
}

и составления списка

        var conditions = new List<Condition>
        {
            new Condition
                {
                    Match = () => formvalues["helmets"] == "" && formvalues["garages"] == "",
                    Action = () =>
                                 {
                                      ViewBag.hideHelmet = 1;    
                                      ViewBag.hideGarages = 1;
                                      ViewBag.SelectedHelmets = 1;
                                      ...
                                 }
                },
            new Condition
                { 
                  ...
                }
         };

затем linq:

   conditions.Where(c => c.Match()).ToList().ForEach( c => c.Execute());

Answer 6

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

void ApplyStrategy(int? helmets, int? garages)
{
 // depends on values of helmets and garages execute appropriate strategy
}

void ApplyNoHelmetsNoGaragesStrategy()
void ApplyNoGaragesStrategy()
void ApplyNoHelmetsStrategy()
// ... 

Answer 7

Я бы использовал Factory Pattern для генерации вашего ViewBag объекта на основе условий.Пусть фабрика сгенерирует желаемое значение ViewBag и назначит для него ваш объект.

Вот пример в примере консоли (просто для быстрой демонстрации):

  class MainApp
  {
     /// <summary>
     /// Entry point into console application.
     /// </summary>
     static void Main()
     {
        ViewBagCreator creator = new ConcreteCreatorForViewBagObject();

        string garages_form_value = ""; // formvalues["garages"];
        string helmets_form_value = ""; // formvalues["helments"];

        // create viewpage
        ViewBagObject view_bag = creator.FactoryMethod(helmets_form_value,
           garages_form_value, 10, 100, 23);
        Console.WriteLine("Created {0}",
          view_bag.GetType().Name);

        // Assign your viewbag object here
        // ViewBag = view_bag;

        // Wait for user
        Console.ReadKey();
     }
  }

  /// <summary>
  /// The 'ViewBagObject' abstract class
  /// </summary>
  abstract class ViewBagObject
  {
     public int hideHelmet;
     public int hideGarages;
     public int SelectedHelmets;
     public int SelectedGarages;
     public int TotalHelmets;
     public int TotalGarages;
     public int TotalAmount;
  }

  /// <summary>
  /// A 'ViewBagNoSelection' class
  /// </summary>
  class ViewBagNoSelection : ViewBagObject
  {
     public ViewBagNoSelection(int trackEventUnitCost)
     {
        hideHelmet = 1;
        hideGarages = 1;
        SelectedHelmets = 1;
        SelectedGarages = 1;
        TotalHelmets = 1;
        TotalGarages = 1;
        TotalAmount = trackEventUnitCost;
     }
  }

  /// <summary>
  /// A 'ViewBagGaragesNoHelments' class
  /// </summary>
  class ViewBagGaragesNoHelments : ViewBagObject
  {
     public ViewBagGaragesNoHelments(int garagesValue, int garagesUnitCost, int trackEventUnitCost)
     {
        hideHelmet = 0;
        hideGarages = 1;
        SelectedHelmets = 1;
        SelectedGarages = garagesValue;
        TotalHelmets = 1;
        TotalGarages = garagesValue * garagesUnitCost;
        TotalAmount = TotalGarages * trackEventUnitCost;
     }
  }

  /// <summary>
  /// A 'ViewBagHelmentsNoGarages' class
  /// </summary>
  class ViewBagHelmentsNoGarages : ViewBagObject
  {
     public ViewBagHelmentsNoGarages(int helmetsValue, int helmentsUnitCost, int trackEventUnitCost)
     {
        hideHelmet = 0;
        hideGarages = 1;
        SelectedHelmets = helmetsValue;
        SelectedGarages = 1;
        TotalHelmets = helmetsValue * helmentsUnitCost;
        TotalGarages = 1;
        TotalAmount = TotalHelmets * trackEventUnitCost;
     }
  }

  /// <summary>
  /// The 'ViewBagCreator' abstract class
  /// </summary>
  abstract class ViewBagCreator
  {
     public abstract ViewBagObject FactoryMethod(
        string helmetsFormValue, string garagesFormValue,
        int helmetsUnitCost, int garagesUnitCost, int trackEventUnitCost);
  }

  /// <summary>
  /// A 'ConcreteCreator' class
  /// </summary>
  class ConcreteCreatorForViewBagObject : ViewBagCreator
  {
     public override ViewBagObject FactoryMethod(
        string helmetsFormValue, string garagesFormValue,
        int helmetsUnitCost, int garagesUnitCost, int trackEventUnitCost)
     {
        bool helmets_value_is_null = (helmetsFormValue == null);
        bool helmets_value_is_empty = (helmetsFormValue == "");
        bool garages_value_is_null = (garagesFormValue == null);
        bool garages_value_is_empty = (garagesFormValue == "");

        int helmets = 0;
        int garages = 0;
        int.TryParse(garagesFormValue, out garages);
        int.TryParse(helmetsFormValue, out helmets);

        bool garages_greater_than_zero = garages > 0;
        bool helmets_greater_than_zero = helmets > 0;

        if (helmets_value_is_empty && garages_value_is_empty)
        {
           return new ViewBagNoSelection(trackEventUnitCost);
        }
        else if (helmets_value_is_empty && garages_greater_than_zero)
        {
           return new ViewBagGaragesNoHelments(garages, garagesUnitCost, trackEventUnitCost);
        }
        else if (garages_value_is_empty && helmets_greater_than_zero)
        {
           return new ViewBagHelmentsNoGarages(helmets, helmetsUnitCost, trackEventUnitCost);
        }
        //...
        return null;
     }
  }

Answer 8

Вы можете абстрагировать ваш ViewBag в функцию, которая устанавливает все ваши данные. Это немного очистит вещи, например,

if(...) {
   SetUpViewBag(...); }
else {
   SetUpViewBag(...); }

...

private SetUpViewBag(...)
{
   ViewBag.SelectedHelmets = prop1;
   ViewBag.SelectedGarages = prop2;
   ViewBag.TotalHelmets = prop3;
   ViewBag.TotalGarages = prop4;
   ViewBag.hideHelmet = prop5;
   ViewBag.hideGarages = prop6;
   ViewBag.TotalAmount = prop7;
}

Answer 9

Вы можете двигаться

ViewBag.hideHelmet = 1;//for jquery, This value is passed to jquery script which then decides which field to hide/show             
ViewBag.hideGarages = 1;//for jquery, This value is passed to jquery script which then decides which field to hide/show             
ViewBag.SelectedHelmets = 1;//             
ViewBag.SelectedGarages = 1;//             
ViewBag.TotalHelmets = 1;             
ViewBag.TotalGarages = 1;             
ViewBag.TotalAmount = 1;             
ViewBag.TotalAmount =  trackEventCost.UnitCost; 

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

  int r = 1;
  long l = r == 1 ? 1 : (r==2 ? 2 : (r==3?3:(r==4 ? 4 : 5)));