views:

365

answers:

7

I've just coded a 700 line class. Awful. I hang my head in shame. It's as opposite to DRY as a British summer.

It's full of cut and paste with minor tweaks here and there. This makes it's a prime candidate for refactoring. Before I embark on this, I'd thought I'd ask when you have lots of repetition, what are the first refactoring opportunities you look for?

For the record, mine are probably using:

  1. Generic classes and methods
  2. Method overloading/chaining.

What are yours?

+2  A: 

#region

I made a 1,000 line class only one line with it!

In all seriousness, the best way to avoid repetition is the things covered in your list, as well as fully utilizing polymorphism, examine your class and discover what would best be done in a base class, and how different components of it can be broken away a subclasses.

FlySwat
#region: the carpet under which to hide your dirty secrets
thijs
+1  A: 

First of all, I would recommend refactoring much sooner than when you are done with the first version of the class. Anytime you see duplication, eliminate it ASAP. This may take a little longer initially, but I think the results end up being a lot cleaner, and it helps you rethink your code as you go to ensure you are doing things right.

As for my favorite way of removing duplication.... Closures, especially in my favorite language (Ruby). They tend to be a really concise way of taking 2 pieces of code and merging the similarities. Of course (like any "best practice" or tip), this can not be blindly done... I just find them really fun to use when I can use them.

Mike Stone
You're quite right about not waiting until the class is finished. I was overtaken by the urge to complete the functionality.
IainMH
+1  A: 

One of the things I do, is try to make small and simple methods that I can see on a single page in my editor (visual studio).

I've learnt from experience that making code simple makes it easier for the compiler to optimise it. The larger the method, the harder the compiler has to work!

I've also recently seen a problem where large methods have caused a memory leak. Basically I had a loop very much like the following:

while (true)
{
  var smallObject = WaitForSomethingToTurnUp();
  var largeObject = DoSomethingWithSmallObject();
}

I was finding that my application was keeping a large amount of data in memory because even though 'largeObject' wasn't in scope until smallObject returned something, the garbage collector could still see it.

I easily solved this by moving the 'DoSomethingWithSmallObject()' and other associated code to another method.

Also, if you make small methods, your reuse within a class will become significantly higher. I generally try to make sure that none of my methods look like any others!

Hope this helps.

Nick

Nick R
+1  A: 

"cut and paste with minor tweaks here and there" is the kind of code repetition I usually solve with an entirely non-exotic approach- Take the similar chunk of code, extract it out to a seperate method. The little bit that is different in every instance of that block of code, change that to a parameter.

There's also some easy techniques for removing repetitive-looking if/else if and switch blocks, courtesy of Scott Hanselman: http://www.hanselman.com/blog/CategoryView.aspx?category=Source+Code&page=2

callingshotgun
+1  A: 

I might go something like this:

Create custom (private) types for data structures and put all the related logic in there. Dictionary<string, List<int>> etc.

Make inner functions or properties that guarantee behaviour. If you’re continually checking conditions from a publically accessible property then create an private getter method with all of the checking baked in.

Split methods apart that have too much going on. If you can’t put something succinct into the or give it a good name, then start breaking the function apart until the code is (even if these “child” functions aren’t used anywhere else).

If all else fails, slap a [SuppressMessage("Microsoft.Maintainability", "CA1502:AvoidExcessiveComplexity")] on it and comment why.

Ant
+2  A: 

I like to start refactoring when I need to, rather than the first opportunity that I get. You might say this is somewhat of an agile approach to refactoring. When do I feel I need to? Usually when I feel that the ugly parts of my codes are starting to spread. I think ugliness is okay as long as they are contained, but the moment when they start having the urge to spread, that's when you need to take care of business.

The techniques you use for refactoring should start with the simplest. I would strongly recommand Martin Fowler's book. Combining common code into functions, removing unneeded variables, and other simple techniques gets you a lot of mileage. For list operations, I prefer using functional programming idioms. That is to say, I use internal iterators, map, filter and reduce(in python speak, there are corresponding things in ruby, lisp and haskell) whenever I can, this makes code a lot shorter and more self-contained.

toby
Yes Fowler's book is really good. Maybe I should dig it out more often! :)
IainMH
+1  A: 

Sometimes by the time you "complete functionality" using copy and paste code, you've come to a point that it is maimed and mangled enough that any attempt at refactoring will actually take much, much longer than refactoring it at the point where it was obvious.

In my personal experience my favorite "way of removing repetition" has been the "Extract Method" functionality of Resharper (although this is also available in vanilla Visual Studio).

Many times I would see repeated code (some legacy app I'm maintaining) not as whole methods but in chunks within completely separate methods. That gives a perfect opportunity to turn those chunks into methods.

Monster classes also tend to reveal that they contain more than one functionality. That in turn becomes an opportunity to separate each distinct functionality into its own (hopefully smaller) class.

I have to reiterate that doing all of these is not a pleasurable experience at all (for me), so I really would rather do it right while it's a small ball of mud, rather than let the big ball of mud roll and then try to fix that.

Jon Limjap