避免针对多个 (9) 条件使用过多的 if else、else if 语句

Question

我的 if else 语句有 9 个条件，我在想是否还有其他替代解决方案可以使代码干净而简短。所有 9 个条件对 asp.net mvc view 执行不同的计算。我已经包含了代码和显示某些条件视图的图像，希望它有意义，我一直在为这种情况寻找更好、更强大的解决方案..谢谢

//Condition 1
//when no selection is made for the 2 dropdownlist (garages & helmets)
if (formvalues["helmets"] == "" && formvalues["garages"] == "")
{
    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;
}
//Condition 2
//When garages are selected from dropdown & helmets are not selected
else if ((formvalues["helmets"] == "") && (Convert.ToInt32(formvalues["garages"]) > 0))
{
    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;
}
//Condition 3
//When helmets are selected from dropdown & garages are not selected
else if ((formvalues["garages"] == "") && (Convert.ToInt32(formvalues["helmets"]) > 0))
{
    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;
}
//Condition 4
//When garages are not selected from dropdown & helmets dropdownlist is hidden on the view due to unavailablity of helmets for that event
else if (formvalues["garages"] == "" && (formvalues["helmets"] == null))
{
    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;
}
//Condition 5
//When helmets are not selected from dropdown & garages dropdownlist is hidden on the view due to unavailablity of garages for that event
else if ((formvalues["garages"] == null) && (formvalues["helmets"] == ""))
{
    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;
}
//Condition 6
//When both helmets and garages dropdown list is not displayed on the view as they are not present in the database
else if (formvalues["helmets"] == null && formvalues["garages"] == null)
{
    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;
}
//Condition 7
//When garages are selected from dropdown & helmets dropdownlist is hidden on the view due to unavailablity of helmets for that event
else if (formvalues["helmets"] == null && Convert.ToInt32(formvalues["garages"]) > 0)
{
    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;
}
//Condition 8
//When helmets are selected from dropdown & garages dropdownlist is hidden on the view due to unavailablity of garages for that event
else if (Convert.ToInt32(formvalues["helmets"]) > 0 && formvalues["garages"] == null)
{
    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;
}
//Condition 9
//When garages and helmet both dropdown are selected
 else
{
    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;
}

Answer 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 ifs，例如：

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

更新（关于您的评论/问题）：

• ? : 语句中的运算符 x = 条件 ? value1 : value2 称为条件运算符。如果 condition 为 true，则返回 value1，否则返回 value2。
• int? 是一个 可空整数，可以是整数值，也可以为空。

Answer 3

为什么不关注任务目标，而关注条件？

条件似乎很简单并且重复。一个示例：

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

这不需要任何分支逻辑。

Answer 4

您可以将 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 5

您可以将

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; 

某些参数移至具有可选参数的方法，因为您不会在所有情况下设置所有值。其次，您可以使用三元运算符。您可以使用三元运算符，例如。

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

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

我将使用 工厂模式 生成您的 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

一些很棒的建议。我喜欢按如下方式处理这些情况。我已经重构了您的代码以完全删除条件。相反，我将每个条件的代码封装到一个方法中，并创建一个具有 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 9

怎么样：

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());