logo标签列表

重构两个几乎具有相同内容的方法

c#refactoring
3

我有两种几乎相同的方法：

public string Method1(int someInt)
{
    if(someBoolean)
        return "test";

    if(someOtherBoolean)
    {
        return "dfjakdsad";
    }
    else
    {
        string myStr = getString(someInt)
    }
}

 public string Method2(myEnum myenum)
 {
    if(someBoolean)
        return "test";

    if(someOtherBoolean)
    {
        return "dfjakdsad";
    }
    else
    {
        string myStr = getString(myenum)
    }
 }

区别在于方法签名和else中的单语句，string myStr = getString

由于Method1被多处调用，因此必须以某种方式保留。我该如何进行重构？

展示 getString() 的定义？ - Denis Palnitsky
如果一个函数被多处调用，重构它只会带来微小的收益，但需要大量测试。除非你有一个有效的更改票据，否则最好不要动它，让它保持原样就好 :-) - Adam Houldsworth
我还建议为了可测试性只有一个出口点。 - Firoso
6个回答
5
我建议使用通用型的：
public string Method<T>(T arg)
{
    if(someBoolen)
        return "test";

    if(someOtherBoolean)
    {
        return "dfjakdsad";
    }
    else
    {
        string myStr = getString(arg)
    }
}

这假设getString本身是通用的或可以处理任何类型的对象。
泛型可能有点过度设计 - 另外更改需要应用于所有调用方，类型需要传递到 getString 中。不过，如果 getString 当前接受对象，泛型的一个好处是消除装箱。 - Adam Houldsworth
@Adam，同一程序集中的调用方不需要进行任何更改，因为编译器可以在调用点推断类型。唯一的困难是来自无法重新编译的外部程序集的调用方，但您可以通过简单的包装器来解决这个问题。 - JSBձոգչ
@JSBangs 我以为只有最新版本的C#才能推断类型？ - Adam Houldsworth
1
@Adam，泛型函数参数的类型推断已经存在了很长时间。你可以像这样调用 Method<T>(T arg) 作为 Method(stringObject)，编译器会自动推断出你的意思是 Method<string>(stringObject)。 - JSBձոգչ
5
如果您的枚举可以转换为整数，并且假设您的getString返回的是枚举的数字值而不是文本，则只需执行以下操作：
public string Method2(myEnum myenum) 
 { 
    return Method1((int)myenum);
 }

作为一个有趣的注解——我曾经看到过这种情况被称为“相同的空格，不同的值”。
同时作为有趣的注解——我的基于头脑的编译器说这段代码实际上无法编译 ：-）
我不会担心重构这个——获得稍微更好的代码可读性和可能的、如果发生变化则降低复杂度—然而这并不是复杂的代码，所以如果它被频繁使用，则额外的测试开销可能会超过收益。 更新：如果枚举getString返回文本，即“Foo”，那么您无法按照我描述的方式重构此代码。
@Adam，在C#中，据我所知所有的枚举类型都是整数。 - CaffGeek
@Chad，默認情況下是的。儘管您可以指定使用哪種基礎類型。 - Charlie Salts
1
@Chad：不是的。默认情况下，enum 是一个整数，但你可以将 enum 定义为任何整数类型（byte、sbyte、short、ushort、int、uint、long、ulong）。 - Adam Robinson
这真的回答了问题吗？如果getString（myEnum）返回“MyEnumValue”，则这是不等同的。它回答了这个问题，如果getString(myEnum)返回枚举的整数值的字符串表示形式。 - btlog
@Adam 抱歉，代码真是太吸引眼球了。这似乎是一个很大的假设。 - btlog
@btlog确实 - 如果情况如此且您的场景正确，则重构无法发生 - 我会修改我的答案。 - Adam Houldsworth
4
您可以将Func作为第二个参数传递。
public string Method(Func<string> myFunc)
{
    if(someBoolen)
        return "test";

    if(someOtherBoolean)
    {
        return "dfjakdsad";
    }
    else
    {
        string myStr = myFunc();
    }
}

Method(myEnum => getString(myEnum));
Method(someInt => getString(someInt));

通常情况下，如果您总是像JSBangs所说的那样调用getString，那么通用程序就是最佳解决方案。
3

这段代码是否是可行的、经过测试的代码？如果是，没有好的理由就不要更改它。我会问自己两个问题：

1）让你的代码变得更短、稍微减少冗余真的值得冒意外引入错误的风险吗？

2）这是我有限的时间、精力和技能可以最好地应用的地方吗？

如果这是你的代码中最糟糕的问题，那么恭喜你，你有很棒的代码。我会在其他地方寻找更大的问题。

1
但是你不觉得这段代码违反了DRY原则，会导致维护上的噩梦吗？因此需要进行重构。 - this. __curious_geek
@this. __curious_geek - 并非所有DRY的违规都会导致噩梦 :-) 我认为，为了避免测试开销而违反DRY原则是可以接受的。 - Adam Houldsworth
1
只是想澄清一下，你的时间和精力有限，而不是你的技能，对吧...？ :-) - gkrogers
@gkrogers：我不知道你怎么样，但我的技能肯定是有限的。 - Eric Lippert
@this. __curious_geek：你的问题已经在我的第一点中回答了。如果减少代码的冗余成本值得冒险，以防意外将其变为不正确的代码，请考虑这样做。但是，如果根本不先提出问题，那就是鲁莽的。 - Eric Lippert
0

在你的情况下，将枚举转换并调用函数的 int 版本是否可行？

public string Method(myEnum myenum)
{
    return Method((int)myenum);
}

public string Method(int someInt) {
    if(someBoolen)
        return "test";

    if(someOtherBoolean)
    {
        return "dfjakdsad";
    }
    else
    {
        string myStr = getString(someInt)
    }
}
如果强制类型转换不起作用，您可以编写一个函数，为给定的枚举值提供适当的整数，然后调用它： return Method(getAppropriateInt(myenum)); - Carl Manaster
0

我会提出Adam答案的相反观点：

枚举是强类型的，而int不是，因此应尽可能使用枚举。因此，我会让“真正”的方法要求一个enum作为参数，并重载一个接受int并将其转换为enum的方法。

请注意，两种方法具有相同的名称，枚举方法排在第一位：这意味着智能感知将首先“建议”使用枚举方法，但显示int作为可用选项：

public string Method1(myEnum myenum)
{
    if(someBoolen)
        return "test";

    if(someOtherBoolean)
    {
        return "dfjakdsad";
    }
    else
    {
        string myStr = getString(myenum)
    }
 }

public string Method1(int someInt)
{
    return Method1((myEnum) someInt);
}

更新： 正如Adam所问，这个解决方案的含义是你总是会传递一个与你的enum成员对应的int。在我的经验中，这是常见的做法（如果枚举只映射到一部分有效的int，那么枚举提供了什么价值？）- 但是值得确保它适合您的用例。

这是否意味着给定的所有int都需要能够转换为该枚举？即，该枚举需要具有这些值。 - Adam Houldsworth
网页内容由stack overflow 提供, 点击上面的
可以查看英文原文，
原文链接