合并相同的功能 [英] Merging Same Functions

问题描述

大家好日子，我只是一名练习程序员。我希望你们中的一些人能教我如何压缩或简化我的代码。

Good day to all, I'm just a practicing programmer. i was hoping if some of you could teach me how could i compress or simplify my codes.

private void txtTime1_TextChanged(object sender, EventArgs e)
        {
            double fixAmount = double.Parse(txtFixAmount.Text);
            fixAmount = 15000 / 26 / 8;
            if (cbType1.SelectedItem.ToString() == "Regular")
            {
                double x = fixAmount * 1.25;
                if (txtTime1.Text.Length > 0)
                {
                    txtAmount1.Text = (x * double.Parse(txtTime1.Text)).ToString();
                }
                else if (txtTime1.Text.Length <= 0 || txtTime1.Text == null)
                {
                    txtAmount1.Text = "0.00";
                }
            }

            else if (cbType1.SelectedItem.ToString() == "Double")
            {
                double x = fixAmount * 2.5;
                if (txtTime1.Text.Length > 0)
                {
                    txtAmount1.Text = (x * double.Parse(txtTime1.Text)).ToString();
                }
                else if (txtTime1.Text.Length <= 0 || txtTime1.Text == null)
                {
                    txtAmount1.Text = "0.00";
                }
            }
        }

        private void txtTime2_TextChanged(object sender, EventArgs e)
        {
            double fixAmount = double.Parse(txtFixAmount.Text);
            fixAmount = 15000 / 26 / 8;
            if (cbType2.SelectedItem.ToString() == "Regular")
            {
                double x = fixAmount * 1.25;
                if (txtTime2.Text.Length > 0)
                {
                    txtAmount2.Text = (x * double.Parse(txtTime2.Text)).ToString();
                }
                else if (txtTime2.Text.Length <= 0 || txtTime2.Text == null)
                {
                    txtAmount2.Text = "0.00";
                }
            }

            else if (cbType2.SelectedItem.ToString() == "Double")
            {
                double x = fixAmount * 2.5;
                if (txtTime2.Text.Length > 0)
                {
                    txtAmount2.Text = (x * double.Parse(txtTime2.Text)).ToString();
                }
                else if (txtTime2.Text.Length <= 0 || txtTime2.Text == null)
                {
                    txtAmount2.Text = "0.00";
                }
            }
        }

i无法上传一个图像，给你一个更广泛的想法。



1.我这里有4件物品

•combobox =（姓名 - cbType1& 2）

•文本框=（名称 - txtTime1& 2）

•文本框=（名称 - txtAmount1& 2）

•文本框=（Name - FixAmount）



2.稍后我会在下面有一个按钮，它将获得txtAmount1和amp;的总和。 txtAmount2





谢谢大家。

i couldn't upload an image to give you a much broader idea.

1. I have 4 items here
• combobox = (Name - cbType1 & 2)
• Textbox = (Name - txtTime1 & 2)
• Textbox = (Name - txtAmount1 & 2)
• Textbox = (Name - FixAmount)

2. later i'll have a button underneath that will get the sum of txtAmount1 & txtAmount2


Thank you all.

推荐答案

你可能已经有了注意到那里有很多相似的代码（并且因为这个原因问了你的问题）。实际上你可以简化它。



在我们开始之前，让我们更正代码中的一个缺陷：

你解析文本输入 txtFixAmount 进入 fixAmount ，并在下一行中指定 15000/26 /的结果8 到 fixAmount 。所以来自 txtFixAmount 的文本根本没有效果。也许您打算将输入值乘以或除以 15000/26/8 的结果？我会假设你想要乘以它。



第一个代码简化：唯一能使if-branch代码不同的东西来自else if-branch是 fixAmount （ 1.25 resp。 2.5 ）的乘数。如果你引入一个新变量（我将在这里称之为乘数），你可以减少依赖于if else-if的代码，将1.25或2.5分配给乘法器。其他一切只是跟随并使用乘数而不是硬编码值。

另外：如果你需要转换值（在这种情况下： cbType1.SelectedItem.ToString（））多次，将其分配给变量（此处为 selectedItem ）并使用该变量在以下代码中：

You probably already have noticed that there is a lot of "similar" code there (and asked your question for that reason). And in fact you can simplify it a lot.

Before we start with that, let's correct one flaw in your code:
You parse the text entered into txtFixAmount into fixAmount and on the following line you assign the result of 15000 / 26 / 8 to fixAmount. So the text from txtFixAmount has no effect at all. Maybe your intention was to multiply or divide the entered value by the result of 15000 / 26 / 8 ? I'll go with the assumption that you wanted to multiply it.

First code-simplification: The only thing that makes the code of the "if"-branch different from the "else if"-branch is your multiplier for fixAmount (1.25 resp. 2.5). If you introduce a new variable (I will call it multiplier here) you can reduce the code that depends on the "if else-if" to assigning either 1.25 or 2.5 to multiplier. And everything else just follows and uses multiplier instead of a hard-coded value.
Also: If you need a converted value (in this case: cbType1.SelectedItem.ToString()) multiple times, assign it to a variable (selectedItem here) and use that variable in the following code instead:

private void txtTime1_TextChanged(object sender, EventArgs e)
{
    double fixAmount = double.Parse(txtFixAmount.Text);
    fixAmount *= 15000 / 26 / 8;

    string selectedItem = cbType1.SelectedItem.ToString();
    double multiplier = 0;

    if (selectedItem == "Regular")
    {
        multiplier = 1.25;
    }
    else if (selectedItem == "Double")
    {
        multiplier = 2.5;
    }

    double x = fixAmount * multiplier;
    if (txtTime1.Text.Length > 0)
    {
        txtAmount1.Text = (x * double.Parse(txtTime1.Text)).ToString();
    }
    else if (txtTime1.Text.Length <= 0 || txtTime1.Text == null)
    {
        txtAmount1.Text = "0.00";
    }
}

下一步：良好做法。



1） double.Parse 如果输入的文本无法解析为double，则会抛出异常。如果您使用 double.TryParse ，您将获得一个布尔结果，指示解析是否成功，这允许您优雅地处理该情况。我们可以将这个应用到你当前使用double.Parse的两行。



2）每当你使用else-if时你也应该得到一个结论else ，因为，在这种情况下，你可能改变你的程序，以便有比常规和双重更多的选项，但可能忘记包括一个额外的else-if来处理那个起初的情况。如果你有一个结论else抛出异常，你将很快意识到你的错误，不必搜索你的代码来找到错误。

Next up: Good practices.

1) double.Parse will throw an Exception if the entered text can't be parsed into a double. If you use double.TryParse instead, you will get a boolean result indicating whether the parsing succeeded or not which allows you to handle that case gracefully. We can apply this to both lines where you currently use double.Parse.

2) Whenever you use an "else-if" you should also have a concluding "else", because, in this case, you might change your program so that there are more options than "Regular" and "Double" but might forget to include an extra "else-if" that deals with that case at first. If you have a concluding "else" that throws an Exception you will become aware of your mistake very quickly and won't have to search through your code to find the bug.

private void txtTime1_TextChanged(object sender, EventArgs e)
{
    double fixAmount;
    if (Double.TryParse(txtFixAmount.Text, out fixAmount))
    {
        fixAmount *= 15000 / 26 / 8;

        string selectedItem = cbType1.SelectedItem.ToString();
        double multiplier;

        if (selectedItem == "Regular")
        {
            multiplier = 1.25;
        }
        else if (selectedItem == "Double")
        {
            multiplier = 2.5;
        }
        else
        {
            throw new NotImplementedException(String.Format("Unrecognized selection: {0}", selectedItem));
        }

        double x = fixAmount * multiplier;
        double time;
        if (Double.TryParse(txtTime1.Text, out time))
        {
            txtAmount1.Text = (x * time).ToString();
        }
        else
        {
            txtAmount1.Text = "0.00";
        }
    }
    else
    {
        txtFixAmount.Text = "0.00";
    }
}

下一代码简化：查看原始代码，您将认识到 txtTime1_TextChanged 和 txtTime2_TextChanged 基本上做同样的事情，只是针对不同的ComboBoxes和TextBoxes。您也可以消除这种事实上的代码重复。为此，您必须用变量替换特定ComboBox和TextBox对象的硬编码使用，并让TextChanged-EventHandler-Methods使用适当的ComboBox和TextBox对象调用该新方法：

Next code-simplification: Looking at your original code, you will recognize that txtTime1_TextChanged and txtTime2_TextChanged essentially do the exact same thing, just for different ComboBoxes and TextBoxes. You can eliminate this de-facto code-duplication as well. For that you have to replace the hard-coded use of specific ComboBox- and TextBox-objects by variables and have your TextChanged-EventHandler-Methods call that new method with the appropriate ComboBox- and TextBox-objects:

private void txtTime1_TextChanged(object sender, EventArgs e)
{
    ProcessTextChanged(cbType1, txtTime1, txtAmount1);
}

private void txtTime2_TextChanged(object sender, EventArgs e)
{
    ProcessTextChanged(cbType2, txtTime2, txtAmount2);
}

private void ProcessTextChanged(ComboBox cbType, TextBox txtTime, TextBox txtAmount)
{
    double fixAmount;
    if (Double.TryParse(txtFixAmount.Text, out fixAmount))
    {
        fixAmount *= 15000 / 26 / 8;

        string selectedItem = cbType.SelectedItem.ToString();
        double multiplier;

        if (selectedItem == "Regular")
        {
            multiplier = 1.25;
        }
        else if (selectedItem == "Double")
        {
            multiplier = 2.5;
        }
        else
        {
            throw new NotImplementedException(String.Format("Unrecognized selection: {0}", selectedItem));
        }

        double x = fixAmount * multiplier;
        double time;
        if (Double.TryParse(txtTime.Text, out time))
        {
            txtAmount.Text = (x * time).ToString();
        }
        else
        {
            txtAmount.Text = "0.00";
        }
    }
    else
    {
        txtFixAmount.Text = "0.00";
    }
}

最后的话：

- 理论上 cbType.SelectedItem 可以为空。在使用 .ToString（）访问它之前检查它是一个好习惯。

- 如果某些输入无法解析，你应该只是默默地将文本重置为0.00，但也向用户显示一些通知，例如一个 MessageBox 。

- 字符串Regular和Double不应该在这里硬编码 - 最好有一个程序中的中心点，用它初始化变量并使用它们来填充你的组合框以及在这里用它们进行比较。



希望它有帮助： - ）

Final words:
- Theoretically cbType.SelectedItem could be null. It would be good practise to check that before accessing it with .ToString().
- In case some input can't be parsed you shouldn't just silently reset the text to "0.00" but also show some notification to the user, e.g. a MessageBox.
- The strings "Regular" and "Double" shouldn't be hard-coded here - it would be better to have one "central" point in your program where you initialize variables with it and use these to fill your ComboBoxes as well as use them for the comparisons here.

Hope it helps :-)

你可以使用sender参数知道谁在调用方法。

例如：

You can use the sender parameter to know who is calling a method.
For example:

private void txtTime_Generic_TextChanged(object sender, EventArgs e)
{
    TextBox tb = (sender as TextBox);
    if (tb != null)  // Just make sure it is a TextBox calling the method
    {
        // Common code
        // ...
        
        if (tb.Name == "txtTime1")
        {
            // Do This
        }
        else if (tb.Name = "txtTime2")
        {
            // Do that
        }
        else
        {
            // Maybe do nothing
        }
    }
}

然后将此方法分配给txtTime1和txtTime2的TextChanged事件。

Then assign this method to the TextChanged events of both txtTime1 and txtTime2.

这篇关于合并相同的功能的文章就介绍到这了，希望我们推荐的答案对大家有所帮助，也希望大家多多支持IT屋！