So I have some code that is pretty ugly, and I want to know if there is a cleaner way to do this. This code is essentially responsible for invoking a selected method for a device. The user will be given a list of available devices, from that chosen device they are given a list of methods (actions the device can perform). Some methods require arguments be gathered by the user, which is where the trickiness begins. In order to actually call on that method I have to invoke it. I can't invoke it unless I have an object array of all the entered arguments with their correct types, but the arguments are ALL gathered as strings from the user. So I have to parse this string and determine what the type of each argument is by comparing it to the type of the parameter at currMethod.GetParameters.
foreach (Control control in panel1.Controls)
{
if (control.GetType().ToString() == "System.Windows.Forms.TextBox")
{
int i = 0;
string paramTypeStr = currMethod.GetParameters()[i].ParameterType.ToString().Replace("&", "");
if (paramTypeStr == "System.Int16")
{
short shortArg = Int16.Parse(control.Text.ToString());
methodArgs.Add(shortArg);
}
else if (paramTypeStr == "System.Int32")
{
int intArg = Int32.Parse(control.Text.ToString());
methodArgs.Add(intArg);
}
else if (paramTypeStr == "System.Double")
{
double doubleArg = Double.Parse(control.Text.ToString());
methodArgs.Add(doubleArg);
}
else if (paramTypeStr == "System.Single")
{
float floatArg = float.Parse(control.Text.ToString());
methodArgs.Add(floatArg);
}
else if (paramTypeStr == "System.Boolean")
{
bool boolArg = bool.Parse(control.Text.ToString());
methodArgs.Add(boolArg);
}
else if (paramTypeStr == "System.Int64")
{
long longArg = long.Parse(control.Text.ToString());
methodArgs.Add(longArg);
}
else if (paramTypeStr == "System.Byte")
{
byte byteArg = byte.Parse(control.Text.ToString());
methodArgs.Add(byteArg);
}
else
{
methodArgs.Add(control.Text.ToString());
}
currMethod.Invoke(DeviceTypeAndInstance.Values.FirstOrDefault(), methodArgs.ToArray());
i = 1;
}
}
If it helps, here is where the arguments are being gathered:
for (int i = 0; i < paramCount; i )
{
TextBox textBox = new TextBox();
textBox.Name = i.ToString();
Label label = new Label();
//a.Text = (i 1).ToString();
textBox.Location = new Point(pointX, pointY);
textBox.Width = 100;
label.Location = new Point(pointX 110, pointY);
label.Text = $"Argument {i}: {currMethodParamArray[i]}"
if (currMethodParamArray[i].ToString() == "System.String profile")
{
textBox.Text = profile;
}
label.Width = 1000;
panel1.Controls.Add(textBox);
panel1.Controls.Add(label);
panel1.AutoScroll = true;
panel1.Show();
pointY = 30;
}
CodePudding user response：
You can make it look much cleaner by using a switch expression (if you're using C# 8.0 and up). Note a switch expression is different from a switch statement.
This is your code as a switch expression:
foreach (Control control in panel1.Controls)
{
if (control.GetType().ToString() != "System.Windows.Forms.TextBox") { continue; }
int i = 0;
string paramTypeStr = currMethod.GetParameters()[i].ParameterType.ToString().Replace("&", "");
dynamic arg = paramTypeStr switch
{
"System.Int16" => Int16.Parse(control.Text.ToString()),
"System.Int32" => Int32.Parse(control.Text.ToString()),
"System.Double" => Double.Parse(control.Text.ToString()),
"System.Single" => float.Parse(control.Text.ToString()),
"System.Boolean" => bool.Parse(control.Text.ToString()),
"System.Int64" => long.Parse(control.Text.ToString()),
"System.Byte" => byte.Parse(control.Text.ToString()),
_ => control.Text.ToString()
};
methodArgs.Add(arg);
i ;
}
You can read more about the switch expression here: https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/operators/switch-expression
CodePudding user response：
Generally, when three or more if statements are querying the same variable data, it is good to use a switch statement. You could separate out the long if statements into a separate method, that could also be formatted in the switch expression syntax if your C# version allows it.
If it were possible, the datatypes would not be strings; for example, by including the selections in a ListView or ComboBox as Type objects. You might also consider using the nameof(Int32) syntax in order to use constants instead of strings.
Without a complete reproducible example of what you are attempting, it's difficult to judge how to improve the code without seeing the interface. Generally though, it's better to create a template (for example, a DataTemplate), along with a template selector, instead of generating the UI from the code-behind. This separates the visuals into markup and the logic into code, which is very helpful.
CodePudding user response：
Use a switch case, The switch statement selects a statement list to execute based on a pattern match with a match expression https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/statements/selection-statements
CodePudding user response：
All these if/else-if statements seem like the perfect opportunity to implement a switch.