开发者

How to simplify choice (if)

I have such a code as to make it better (modbus_master.SetValue("x1", Convert.ToInt32(resipeDosings[i].Massa) * 10, 1); - sending data to the controller)

 public class RecipeDosings
 {
    public string Product { get; set; }
    public string Persent { get; set; }
    public string Massa { get; set; }
    public string Bunker { get; set; }

    public RecipeDosings(string product, string persent, string massa, string bunker)
    {
        this.Product = product;
        this.Persent = persent;
        this.Massa = massa;
        this.Bunker = bunker;
    }
  }

 public List<RecipeDosings> resipeDosings = new List<RecipeDosings>();

        for (int i = 0; i < resipeDosings.Count; i++)
        {
            if (resipeDosings[i].Bunker == "Bunker 1")
            {
                modbus_master.SetValue("x1", Convert.ToInt32(resipeDosings[i].Massa) * 10, 1);      
            }
            if (resipeDosings[i].Bunker == "Bunker 2")
            {
                modbus_master.SetValue("x1", Convert.ToInt32(resipeDosings[i].Massa) * 10, 1);
            }
            if (resipeDosings[i].Bunker == "Bunker 3")
            {
                modbus_master.SetValue("x1", Convert.ToInt32(resipeDosings[i].Massa) * 10, 1);
            }
            if (resipeDosings[i].Bunker == "Bunker 4")
            {
                modbus_master.SetValue("x1", Convert.ToInt32(resipeDosings[i].Massa) * 10, 1);
            }
            if (resipeDosings[i].Bunker == "Bunker 5")
            {
                modbus_master.SetValue("x1", Convert.ToInt32(resipeDosings[i].Massa) * 10, 1);
            }
            if (resipeDosings[i].Bunker == "Bunker 6")
            {
                modbus_master.SetValue("x1", Convert.ToInt32(resipeDosings[i].Massa) * 10, 1);
            }
            if (resipeDosings[i].Bunker == "Bunker 7")
            {
                modbus_master.SetValue("x1", Convert.ToI开发者_如何学Cnt32(resipeDosings[i].Massa) * 10, 1);
            }
            if (resipeDosings[i].Bunker == "Bunker 8")
            {
                modbus_master.SetValue("x1", Convert.ToInt32(resipeDosings[i].Massa) * 10, 1);
            }
            if (resipeDosings[i].Bunker == "Bunker 9")
            {
                modbus_master.SetValue("x1", Convert.ToInt32(resipeDosings[i].Massa) * 10, 1);
            }
        }


Switch statement removes all the if statements -

switch (resipeDosings[i].Bunker)
{
case "Bunker 1":
    // code here
    break;
case "Bunker 2":
    // code here
    break;

    // repeat case statements...

default:
    // this is the final 'else' code - if nothing matches
}

However, two things are obvious:

  • You're executing the same code regardless
  • You should probably store the variables (something that might be different for each Bunker) in a look-up table or database table, so you don't need to modify the program each time you get a new Bunker or want to change a value

The easiest way of building a LUT is using a Dictionary<>.

Dictionary<string, int> bunkerLut = new Dictionary<string, int>();

bunkerLut["Bunker 1"] = 10;
bunkerLut["Bunker 2"] = 11;
bunkerLut["Bunker 3"] = 12;

// and so on... I'm assuming there should be a value that's different for each bunker?  I'm also assuming it's an int

Then you can look up, something like this (assuming the 10 is the value that changes):

int result = bunkerLut[resipeDosings[i].Bunker];
modbus_master.SetValue("x1", Convert.ToInt32(resipeDosings[i].Massa) * result, 1);

Other options are storing the values in a configuration file or database.


You can do the following:

for (int i = 0; i < resipeDosings.Count; i++)
{
    switch(resipeDosings[i].Bunker)
    {
        case "Bunker 1":
        case "Bunker 2":
        case "Bunker 3":
        case "Bunker 4":
        case "Bunker 5":
        case "Bunker 6":
        case "Bunker 7":
        case "Bunker 8":
        case "Bunker 9":
        case "Bunker 10":
            modbus_master.SetValue("x1", Convert.ToInt32(resipeDosings[i].Massa) * 10, 1);
            break;
        default:
            break;
    }
}


Uhm, as long as there is no difference in how you treat them:

public List<RecipeDosings> resipeDosings = new List<RecipeDosings>();

for (int i = 0; i < resipeDosings.Count; i++)
{
    if (resipeDosings[i].Bunker.StartsWith("Bunker "))
    {
        modbus_master.SetValue("x1", Convert.ToInt32(resipeDosings[i].Massa) * 10, 1);      
    }
}


Since there seems to be no difference in what is done for the different values, you can just verify that it is one of the values to check for, and perform the action if that is the case:

var valuesToCheck = new[] { 
    "Bunker 1",
    "Bunker 2",
    "Bunker 3",
    "Bunker 4",
    "Bunker 5",
    "Bunker 6",
    "Bunker 7",
    "Bunker 8",
    "Bunker 9"};

for (int i = 0; i < resipeDosings.Count; i++)
{                
    if (valuesToCheck.Contains(resipeDosings[i].Bunker)
    {
        modbus_master.SetValue("x1", Convert.ToInt32(resipeDosings[i].Massa) * 10, 1);      
    }    
}


I could be overlooking something, but there is no difference for each bunker case.

for (int i = 0; i < resipeDosings.Count; i++)
    modbus_master.SetValue("x1", Convert.ToInt32(resipeDosings[i].Massa) * 10, 1);

This would do exactly the same.


One more thing:
If you use if in the way you do, i.e. with checks of which only one can be true at a time, use else if, so it will not evaluate expressions that are known to be false:

if(cond1)
{
   // do stuff
}
else if(cond2)
{
   // do stuff
}
else if(cond3)
{
   // do stuff
}

But do so only, if there is no better alternative like switch or something else...


I'd have a dictionary of string/func, something like this;

class EmirateRefactored
{
    public List<RecipeDosings> resipeDosings = new List<RecipeDosings>();
    private Dictionary<string, Func<RecipeDosings, int>> m_objActionMapping;

    public EmirateRefactored()
    {
        m_objActionMapping = new Dictionary<string, Func<RecipeDosings, int>>();
        m_objActionMapping.Add("Bunker 1", bunker1);
        m_objActionMapping.Add("Bunker 2", bunker2);
        //etc
    }

    public void Process(ModBusMaster modbus_master)
    {
        foreach (RecipeDosings rd in resipeDosings)
        {
            if (m_objActionMapping.ContainsKey(rd.Bunker))
            {
                int result = m_objActionMapping[rd.Bunker].Invoke(rd);
                modbus_master.SetValue(result);
            }
            else
                throw new ApplicationException("Couldn't parse bunker!");
        }
    }

    /// <summary>
    /// I have no idea what this is as it's not included in the sample code!
    /// </summary>
    public class ModBusMaster
    {
        public void SetValue(int i_intInput) { }
    }

    /// <summary>
    /// Some code relevant to "bunker 1"
    /// </summary>
    private int bunker1(RecipeDosings i_objRecipe)
    {
        return Convert.ToInt32(i_objRecipe.Massa) * 10;
    }
    /// <summary>
    /// Some code relevant to "bunker 2"
    /// </summary>
    private int bunker2(RecipeDosings i_objRecipe)
    {
        //bunker2 code
        return Convert.ToInt32(i_objRecipe.Massa) * 10;
    }
}

I'm making several assumptions, and have no idea what the actual requirements are without more sample-code / example matches, etc, etc.

Edit: After reading some comments on other answers, you could quite easily modify this to a dictionary of Regex/func. And from the process method; if the "Bunker" property matches the regex Invoke the func.

0

上一篇:

下一篇:

精彩评论

暂无评论...
验证码 换一张
取 消

最新问答

问答排行榜