What is the best way to avoid duplicating this code

Question

I have the following methods:

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 ; } } 

What is the best way to avoid duplication here and provide as much code as possible

Answer 1

How to use LINQ and Sum extension method?

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

If this is not an option, you can reorganize the logic into your own sum method:

 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 ; } 

A note about the last example: I compiled the type "Row", as this was not visible from your example. Replace the appropriate type.

Answer 2

Do not worry.

You can shorten the code, but if you try to reorganize it to eliminate duplication, you just make the code more complex.

Concentrate your refactoring efforts on more complex and worthy cases. Life is too short ...

A few bytes of disk space are cheap as a last resort, and Copy / Paste is the developer's best friend. Also, if we are purists about this, consider the overhead of your refactoring time.

Answer 3

This is probably not the answer you are looking for, but it is not necessarily duplication. It is sometimes misunderstood that if two different functions look the same, they must be reorganized to remove the duplicate. In my humble opinion, THIS IS WRONG.

This is only duplication if they really are replication and an identical or almost identical concept. For this to be duplication (at least the duplication that needs to be removed), it’s not just that they use the same code, but use the same code for the same reason.

It might just be because of the sample you sent, but it was simple enough. Although the implementation of the two properties was identical, there must be some valid business reason that you have these two properties, otherwise the simplest answer is to remove the second property together. However, if you really have two properties, and they just look the same now, this does not mean that they will not diverge in functionality in the future (and this is normal).

The idea is to minimize the complexity and cost of maintenance. You can only do this if your code makes sense and simulates reality, but not if it introduces false comparisons, combining things that just look similar.

http://mooneyblog.mmdbsolutions.com/index.php/2010/07/30/reusable-code-is-bad/

Answer 4

I would not worry about the properties personally, just called directly from the outside using linq

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

Answer 5

It is pretty clean. If I knew the type of the string, I would be inclined to put PropertyN () in my class, not here, but did not have this knowledge:

 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; } 

Answer 6

give it an input parameter that says which parameter

 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 ; } } 

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 ; } 

in the pribvate method, I used the object in the signature - until I have the best knowelege type needed for the property