【问题标题】:Is there any reason not to refactor this, making it simpler?有什么理由不重构它,让它更简单吗?
【发布时间】:2013-09-18 21:20:43
【问题描述】:

我在旧代码库中遇到了这个问题:

string[] deptNo = UPC.getDept().Split(new Char[] {'.'});

...并认为这会更简单:

string[] deptNo = UPC.getDept().Split('.');

...但如果我进行此更改,如果点的特征有可能产生某种“魔法”,导致整个意大利面屋滑落,则不想更改它。

更新

作为对给出的警告的回应:是的,我知道您的意思是,在吃旧意大利面时需要极其谨慎地进行操作。对这段代码看似无害的更改:

private void comboVendor_SelectedValueChanged(object sender, EventArgs e)
{
    try
    {
        if ((comboVendor.SelectedIndex >= 0) && (cmbVendor.Text != comboVendor.Text))
        {
            string t = comboVendor.Text; 
            t = t.Substring(0,maxVendorChar);
            t = t.Substring(0,substrLen);
            t = t.Trim();
            cmbVendor.Text = t;
            cmbVendor.Focus();
        }
    }
    catch (Exception ex)
    {
        Duckbill.ExceptionHandler(ex, "frmInv.comboVendor.SelectedValueChanged");
    }               
}

...即,将“Trim()”附加到“.Text”的实例,导致错误消息“指定的参数超出了有效值的范围。”因为“maxVendorChar”不大于被子字符串的值。所以我不得不把它改成这样才能让它工作:

private void comboVendor_SelectedValueChanged(object sender, EventArgs e)
{
    int substrLen;
    try
    {
        if ((comboVendor.SelectedIndex >= 0) && (cmbVendor.Text.Trim() != comboVendor.Text.Trim()))
        {
            string t = comboVendor.Text.Trim(); 
            substrLen = GetLowerInt(t.Length, maxVendorChar);
            t = t.Substring(0,substrLen);
            t = t.Trim();
            cmbVendor.Text = t;
            cmbVendor.Focus();
        }
    }
    catch (Exception ex)
    {
        Platypus.ExceptionHandler(ex, "frmInv.comboVendor.SelectedValueChanged");
    }               
}

private int GetLowerInt(int first, int second)
{
    return first < second ? first : second;
}

在远处,我听到一只乌鸦在“再也没有”的叫声,但我不确定他在说什么。

更新 2

每当人们注意到这个项目是多么的僵硬,他们几乎总是建议我重构它;然而,如上所述,有时一点点重构就会引发一系列震耳欲聋的爆炸,让我希望自己已经病得够重了。真正的解决方案是用更好的方法和至少不那么古老的技术完全重写这个项目。这就是我的愿景;不过,就目前而言,维护模式似乎已成为主流。

【问题讨论】:

  • 这两个调用约定在 C# 中使用 params 关键字时是相同的,Split() 就是这样做的。

标签: c# refactoring simplify


【解决方案1】:

如果您正在处理“一大团泥球”,那么最好不要管它(假设它的行为正确)。

【讨论】:

  • “大泥球”是轻描淡写的;这段代码是由几个人在几年内编写的,他们都有不同的风格和不同的技能水平。就好像贾斯汀·比伯、Lady Gaga、湾城滚轮队、特罗格斯、约翰·科尔特兰和 J.S.巴赫在没有排练的情况下全部即兴演奏。
【解决方案2】:

取决于您是在寻找哲学原因还是技术原因。

没有技术原因。没有魔法,两种说法是一样的。 split 的方法参数使用 params 关键字来允许可变数量的参数语法:

public string[] Split(params char[] separator)

有关将 params 关键字添加到方法参数的目的,请参阅此讨论:Why use the params keyword?

从哲学上讲,重构它可能没有任何真正的好处。作为一般规则,您可能不想仅仅因为可以清理,尤其是没有测试。

【讨论】:

  • 关于单元测试的好主意(每次我对这段代码进行更改时,我都会捂住头,潜入桌子底下,闭上眼睛,想想小熊维尼)。但是:我无法在桌面上运行此应用程序 - 我必须在 XP 模式下编译它,将其(以及同一解决方案中的辅助 DLL)复制到我的 Windows 7 驱动器的文件夹中,然后将其从那里复制到手持设备,并在那里运行它。我不能在手持设备上添加更多东西,因为在吃热狗比赛之后,它已经比小林健装得更满了。关于如何在我的困境中实施测试有什么建议吗?
  • 这是非常难的问题之一——没有通用解决方案。自动化测试的目标是在您的开发环境中进行。有很多技术,但我会尝试发布另一个问题,给出一些带有正确标签的细节以获得最佳答案。这个问题太大了,无法发表评论。
【解决方案3】:

听起来您需要一些单元测试来让您对重构代码有必要的信心。最佳做法是在重构之前添加它们。

我们看不到 Split() 方法的方法签名,但如果它使用可变参数,那么您的重构在理论上是安全的。

【讨论】:

  • 什么是“可变参数”?可能是 var args 吗?
【解决方案4】:

假设getDept() 返回一个字符串,第一种方法的优点是包含新字符作为拆分器更容易。您只需将它们添加到 char 数组中。第二种方法修复了分隔符。 在第二种方法中,'.'也是一个字符,不像“.”,它是一个字符串。 如果getDept() 方法返回一个自定义类,该类的方法是Split(char[] splitters),除非该方法有Split(char splitter) 的重载,否则您将遇到麻烦。 在完全不相关但重要的一点上,实现版本控制系统可能是一个好主意,这样当出现故障时,您可以“重新滚动”更改(即返回到以前的“提交”)。

【讨论】:

  • 我们正在使用版本控制,虽然我不能说它工作得很好(IOW:我不相信它,并且经常保存文本文件并将它们命名为 frmEntry_20130918.txt 等。我经常在我的同伙已经签出文件时签出文件,但系统没有告诉我(XP 模式下的 TFS,Visual Studio 2003)。
  • 我明白了。那么使用自己的方式总是更好的选择
【解决方案5】:

我认为重构此代码没有任何价值,因为我在这里看不到该工作的任何附加值。

此外,我也会保留当前代码,因为它允许我在未来添加更多分隔符。

【讨论】:

    猜你喜欢
    • 1970-01-01
    • 1970-01-01
    • 1970-01-01
    • 2015-07-24
    • 2011-11-20
    • 1970-01-01
    • 2012-03-06
    • 2013-08-12
    • 1970-01-01
    相关资源
    最近更新 更多