如何避免代码重复的最佳方法？

Question

我有以下几种方法：

    public  int CountProperty1
    {
        get
        {
            int count = 0;
            foreach (var row in Data)
            {
                count = count + row.Property1;
            }
            return count ;
        }
    }

    public  int CountProperty2
    {
        get
        {
            int count = 0;
            foreach (var row in Data)
            {
                count = count + row.Property2;
            }
            return count ;
        }
    }

在这里避免重复并尽可能共享代码的最佳方法是什么？

- leora

---

值得一提的是，作为旁注，你的实现是一个Sum()，而不是Count()吗？ - Steve Morgan

Answer 1

使用LINQ和Sum扩展方法怎么样？

public int CountProperty1 
{
    get { return Data.Sum(r => r.Property1); }
} 

如果那不是一个选项，你可以将逻辑重构到你自己的求和方法中：

public  int CountProperty1
{
    get
    {
        return CountProperty(r => r.Property1);
    }
}

public  int CountProperty2
{
    get
    {
        return CountProperty(r => r.Property2);
    }
}

private int CountProperty(Func<Row,int> countSelector)
{
     int count = 0;
     foreach (var row in Data)
     {
         count = count + countSelector(row);
     }
     return count ;
}

关于最后一个例子的说明：我编了一个“Row”类型，因为从你的例子中无法得知。请替换为正确的类型。

- driis

---

这样做的优点在于代码量减少，因此重复不再是一个问题。 - Schroedingers Cat

快速计算一下，您会发现您重构后的代码行数与原始代码完全相同，但执行更为复杂且较慢。这并不是重构带来的好处的典型例子！（不包括 Data.Sum() 建议，该建议非常出色） - Steve Morgan

@Steve，我也喜欢Sum()解决方案，我只是想提供一个更一般化的重构问题的示例 - 另外，在他的代码库中可能不止有两个看起来相同的属性。 - driis

Answer 2

别费心了。

你可以缩短代码，但如果试图重构以消除重复，你只会让代码更加复杂。

把你的重构精力集中在更复杂和值得的情况上。人生太短暂了...

一点磁盘空间的成本非常便宜，复制/粘贴是开发者最好的朋友。此外，如果我们对此保持纯粹主义态度，考虑一下重构的运行时开销。

- Steve Morgan

---

我强烈不同意。虽然在某些情况下你的观点可能是正确的，但这是非常明显的代码重复，可以轻松提取到一个简单的辅助程序中，因此值得去除重复。 - driis

我的观点是，尽管代码重复，但它很琐碎。你的CountProperty方法更加复杂，只比它少了几行代码。简洁和冗长之间的权衡并不总是直截了当的。 - Steve Morgan

@Steve - 我完全同意你的观点 - 不过，我认为这些应该是方法而不是属性。 - Barry Kaye

Answer 3

也许这不是您要找的答案，但这并不一定是重复。有时会误解，如果两个不同的函数看起来相同，就应该重构以消除重复。在我谦虚的意见中，这是错误的。

只有当它们真正复制和相同或几乎相同的概念时才是重复。要使其成为重复（至少应该删除的重复），不仅仅是因为它们恰好使用相同的代码，而是因为它们出于相同的原因使用相同的代码。

可能仅因为您发布的示例足够简单。尽管两个属性的实现相同，但必须有一些有效的业务原因使您拥有这两个属性，否则最简单的答案是完全删除第二个属性。但是，如果您确实有两个属性，并且它们现在恰好看起来相同，那并不意味着它们将来不会分化功能（这是可以接受的）。

这个想法是最小化复杂性和维护成本。只有在您的代码有意义且符合现实模型的情况下，您才能做到这一点，但如果它通过将仅仅看起来相似的事物聚集在一起引入了错误的比较，则无法实现这一点。 http://mooneyblog.mmdbsolutions.com/index.php/2010/07/30/reusable-code-is-bad/

- Mike Mooney

Answer 4

个人认为不必使用属性，只需在使用linq时直接调用即可。

myObject.Data.Sum(x=>x.Property1)

- Paul Creasey

---

你需要将每个对CountProperty1属性的调用都替换为Data.Sum(x=>x.Property1)。 - Evren Kuzucuoglu

Answer 5

这很干净。如果我知道行的类型，我会倾向于将PropertyN()放入其类中而不是在此处，但缺乏该知识：

public int CountPropertyN(int n)
{
    int count = 0;
    foreach (var row in Data)
    {
        count = count + PropertyN(row, n)
    }
    return count ;
}

private int PropertyN(var row, int n) 
{
     if (n == 1) return row.Property1;
     else return row.Property2;
}

- Carl Manaster

Answer 6

给它一个输入参数，指定哪个参数

public  int CountProperty (int whichProperty) 
    {
        get
        {
            int count = 0;
            foreach (var row in Data)
            {
                if( whichProperty = 1)
                    count = count + row.Property1;
                if( whichProperty = 2)
                    count = count + row.Property2;
            }
            return count ;
        }
    }

- Demian Kasier

---

似乎你只是在转移问题而不是解决它...虽然我没有给你点踩。 - musefan

Answer 7

public  int CountProperty1
{
    get
    {
        return GetCount(row.Property1);
    }
}

public  int CountProperty2
{
    get
    {
        return GetCount(row.Property2);
    }
}

private int GetCount(object property)
{
    int count = 0;
    foreach (var row in Data)
    {
        if(property == row.Property1)
        {
            count = count + row.Property1;
        }
        else if (property == row.Property2)
        {
            count = count + row.Property2;
        }
    }
    return count ;
}

在私有方法中，我在签名中使用了对象 - 直到我对属性所需的类型有更好的了解。

- Adam Stewart