如何优化这段代码？

Question

肯定有很多方法可以优化以下代码，我基本上需要确保很多文本框不为空，并读取它们的值：

if (foo1.Text.Length != 0 & bar1.Text.Length != 0)
{
    output.Text += myStrings[i] + " / " + foo1.Text + " / " + bar1.Text;
}

if (foo2.Text.Length != 0 & bar2.Text.Length != 0)
{
    output.Text += myStrings[i] + " / " + foo2.Text + " / " + bar2.Text;
}

if (foo3.Text.Length != 0 & bar3.Text.Length != 0)
{
    output.Text += myStrings[i] + " / " + foo3.Text + " / " + bar3.Text;
}

if (foo4.Text.Length != 0 & bar4.Text.Length != 0)
{
    output.Text += myStrings[i] + " / " + foo4.Text + " / " + bar4.Text;
}

if (foo5.Text.Length != 0 & bar5.Text.Length != 0) 
{
    output.Text += myStrings[i] + " / " + foo5.Text + " / " + bar5.Text;
}

if (foo6.Text.Length != 0 & bar6.Text.Length != 0)
    output.Text += myStrings[i] + " / " + foo6.Text + " / " + bar6.Text;

if (foo7.Text.Length != 0 & bar7.Text.Length != 0)
{
    output.Text += myStrings[i] + " / " + foo7.Text + " / " + bar7.Text;
}

if (foo8.Text.Length != 0 & bar8.Text.Length != 0)
{
    output.Text += myStrings[i] + " / " + foo8.Text + " / " + bar8.Text;
}

if (foo9.Text.Length != 0 & bar9.Text.Length != 0)
{
    output.Text += myStrings[i] + " / " + foo9.Text + " / " + bar9.Text;
}

if (foo10.Text.Length != 0 & bar10.Text.Length != 0)
{
    output.Text += myStrings[i] + " / " + foo10.Text + " / " + bar10.Text;
}

- BeafTurkey

Answer 1

我会将重复的元素放在数组中，然后循环遍历。

TextBox[] foos = new TextBox[] { foo1, foo2, foo3, /* etc */ };
TextBox[] bars = new TextBox[] { bar1, bar2, bar3, /* etc */ };

for (int i = 0; i <= 10; i++)
    if (foos[i].Text.Length != 0 && bars[i].Text.Length != 0)
        output.Text += myStrings[i] + "/" + foos[i].Text + bars[i].Text;

当然，如果元素的名称是按顺序命名的，您可以通过从表单的Controls集合中查找名称为“foo”+ number.ToString()的控件来填充数组。

- rix0rrr

---

这是一个非常好的解决方案！而且元素按顺序命名，使得它更加容易。谢谢！ - BeefTurkey

一般来说，我不喜欢这种方法，因为foos和bars之间存在隐含的、不明显的关联。我建议通过创建一个FooBar类并创建一个List<FooBar>来明确表示它们之间的关联，以便后续使用。 - Greg D

Answer 2

我会遍历这些TextBox所在的Controls集合并筛选出仅为TextBox的控件，然后执行检查和连接。同时，我强烈建议使用StringBuilder而不是+=。

- Gerrie Schenck

---

+1 for StringBuilder。在这里，问题中的字符串连接将是主要的低效率，而且在它出现的程度上，甚至可能会被用户注意到。 - Jeffrey L Whitledge

是的，你关于 StringBuilder 的想法可能是正确的。目前，在生成所有字符串之前，它实际上需要花费很多秒钟的时间。感谢你的建议！ - BeefTurkey

Answer 3

有很多方法可以重构这个代码。你选择的方法将取决于你的特定情况和需求。

1. 创建一个函数，它以foo和bar作为参数并返回字符串，然后在字符串生成器中聚合该字符串。
2. 将foos和bars放入集合中并循环遍历这些集合。在这种情况下，数组将非常有用，并提供一种相互索引数组的方式。
3. 将第2步进一步发展，创建一个新类FooBar，它将foo和bar一起保存。这样，您就可以创建一个FooBars集合，而且他们之间不再有隐含的关联，现在是显式和规范化的。
4. 将第3步进一步发展，认识到你正在聚合一个字符串。如果您使用最近版本的c#，请利用LINQ(.Select().Aggregate())中的Map/Reduce将您的FooBars转换为相应的字符串，然后将字符串聚合成输出。

这只是我头脑中的一些想法。如果你更努力地工作，我相信你会做得更好。 :)

（如果这是家庭作业，请添加一个家庭作业标签。）

编辑：

基于你在另一篇帖子中的评论，指出连接字符串需要“很多秒”，我不禁对UI设计产生疑问。10个字符串本身并不需要太多时间，但这表明您的外部循环（生成i）运行时间相当长。

如果你有自由做出这样的决定，你确定你的UI实际上对于手头的任务是好的吗？一大堆文本框是一个普遍困难的用户界面。也许ListView会更合适。它将隐含包含一个集合，因此您无需处理这个“十个文本框”的愚蠢，并且对于您的用户来说，它将是一个更容易理解的UI。

- Greg D

---

我认为你对ListView可能有所发现。事实上，我将重新编码以实现ListView元素，而不是一组固定的文本框。感谢你的提示。 - BeefTurkey

Answer 4

编写一个接受foo和bar类型的函数。将所有foo1和bar1传递到foo10和bar10以获得值。您也可以创建foo和bar的数组，然后循环调用该方法来获取字符串。

- Vinay

Answer 5

 foreach (Control ctrl in Page.Controls) {
      if (ctrl is TextBox) {
          if (ctrl.Text.Length != 0) {
              output.Text += myStrings[i] + "/" + ctrl.Text;
           }
      }
 }

未经测试，但应该可以工作。使用此方法，您的文本框可以命名为任何名称。

- Learning

Answer 6

你能把foo1-foo10和bar变成一个数组，foo[10]和bar[10]吗？这样可以用一个简单的循环来表达。

- Sol

Answer 7

是否可行制作一个名为foo和bar的文本框的WebControl，并具有以下功能：

if (foo.Text.Length != 0 & bar.Text.Length != 0)
    return myStrings[i] + " / " + foo.Text + " / " + bar.Text;
else
    return string.Empty;

将这十个放在您的页面上，然后使用：

output.Text = myControl1.FooBar + myControl2.FooBar + myControl3.FooBar + ...

（还是有点乱，但不再那么重复。）

- teedyay