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;
}
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.