[red_4900]

From what I've read, you're supposed to refactor your code when you find code smell in your program. But even then, some junior programmer (like me) might not be able to decide which parts of the program needs refactoring, while those that has worked on the same system for a few years might be able to identify directly which parts needs to be refactored. It sounds like it all comes down to experience, but really, is there any trick you would like to recommend to junior programmer how to detect code smells early? Is there any substitute for experience when it comes to refactoring?

Even then, when you do identify code smells, how do you know which parts of the program needs to be refactored first? How do you know which combination of refactoring will bring you the best performance/time-efficiency(if you're working in a time constraint)/cost-efficiency?

I would like to ask for you to share your thoughts and experience.

This post has been edited by red_4900: 15 May 2010 - 09:50 AM

[Martyr2]

Look for repeating code. If you see the exact same series of lines appear together then you have a quick easy refactor choice there that says "take out repeating lines from the multiple areas, put it into its own function and then call the function from multiple locations. Here is an example...

function example1() {
   int a = 1;
   int b = 2;
   int c = a + b;
   return c;
}

function example2() {
   int a = 1;
   int b = 2;
   int c = a + b;
   int d = c + 3;
   return d;
}

Notice that both functions have a series of statements that are exactly the same. In this case the lines where we declare a, b and c. Since these three statements are the same in both locations, they are redundant and perfect for a quick refactor.

function example1() {
   return getCValue(); // <-- Notice we call our refactored function now
}

function example2() {
   int d = getCValue() + 3; // <-- Again we call our refactored function
   return d;
}

// Refactored to contain repetitive statements ONCE. Easier to maintain
// because we can change it here instead of finding all other examples which 
// would need the change.
function getCValue() {
   int a = 1;
   int b = 2;
   int c = a + b;

   return c;

The idea here is that we took the repetitive lines and isolated them into their own function. This cuts down the reptition and makes it easier to later modify because we modify our statements in the getCValue() function once instead of trying to find all functions which have the same three lines and having to edit them all independently. What if we missed example2() function? There is now going to be a bug.

This is one of the easiest to identify and always a welcomed refactoring choice.

[red_4900]

Ahh~ the extract method. Always nice to have and nice to do.

But how about when you all kinds of data clumped into one place? As Martin Fowler stated in his book here, you could deal this by extracting class, introduce parameter object, or preserve the whole method. But how do you decide which one should be done first? How do you decide which one would be the most cost-efficient things to do?

[baavgai]

I love Fowler's stuff. Your primary goal is simplicity. In complex systems, simplicity is hard to find.

red_4900, on 16 May 2010 - 03:06 AM, said:

How do you decide which one would be the most cost-efficient things to do?

It not cost, it's maintainability. To some extent, this is the art of programming. Organizing code in an aesthetically pleasing way that's balanced and clear. There is no single correct answer to that.

Think of just writing a single function. You bang it out, you get rid of the declarations you didn't use, maybe change up variable names, put lines is more consistent order, etc. You probably do this on the small block level all the time. Refactoring is the same thing, only larger.

Once you've got a method working, you step back. Do two methods do very similar things? Can you abstract a third method and have those two simply call it with parameters? Now think of the same process on a class level. Do two classes share code? Can that code logically belong to a base class?

Remember that refactoring can be premature optimizing, which isn't good. That's why you wait until you have a whole mess of code in front of you before you worry about it. You need enough elements in play so that changes make sense of the whole project, rather than a narrow element that actually isn't that general.

In a broad sense, refactoring is like editing a manuscript. You're going over the work again, looking for awkward turns of phrase, similar ideas that can be more elegantly stated together, other passages that should be expanded upon. There is no correct answer, only what scans best to the reader.

[cfoley]

My rules of thumb are to refactor when:

The code is so messy I can't understand the bit I'm trying to debug or add to.

The code is organised in a way that makes it difficult to make a change or add functionality.

I also refactor when I have some spare time, though I should really be doing something directly productive instead. Refactoring is most powerful when it is done with a goal in mind. When I spend time refactoring without a goal, there is a (small) risk of making it harder to include next week's feature.

[pbattisson]

I often find the key with refactoring is little and often, putting it alongside a sort of formal method. A brilliant example of doing this with Test Driven Development can be found here. By programming in a way that makes you have to think about refactoring you are more likely to do so and produce nice clean code.

It is often a lot harder to do this in a legacy system (which it sounds like you are doing) due to the fact that you are changing code you have not written and may have not fully understood. In this case try to make the change, run all of the unit tests and see if you get any failures. A good unit test setup can aid you in seeing when refactoring fails to improve your code. If you are working on a legacy system and finding it hard to refactor, try just doing a quick refactor every time you add some code.

Hope that helps.

[MentalFloss]

It might not be what you're after. It also might be "wrong" in some aspects.

However, I am working on a project that lets you see exactly how I am refactoring. You should be able to take the concepts and get an idea of why I am doing what I'm doing.

Anyway, it's in C#. The code base isn't mine but that makes it more fun. Also, if you intend to try it out, start at the first entry and work through it. Each update is pretty simple in itself but taken as a whole will give you some ideas about it.

http://www.dreaminco...hess-adventure/

[Bacanze]

Also, if you want to know more about the topic, here is a very popular book regarding refactoring.

I picked this up from my uni library yesterday after hearing about it on this forum, I've only had time to quick look through, but it looks great!

http://www.amazon.co...y/dp/0201485672

It gets good IMO about half way through, although as mentioned I've only spent about 20 minutes looking through it due to exam constraints.

[red_4900]

Bacanze, it was that book that I was reading that brings me to this topic. As baavgai said, Your primary goal is simplicity, which brings me to one question. If simplicity is the goal, instead of refactoring depending on the code smell like Fowler stated in his book, why don't we refactor code based on software metrics? Doesn't software metric like cyclomatic complexity gives us the information on when and where to refactor?

This post has been edited by red_4900: 05 June 2010 - 01:36 PM

[baavgai]

red_4900, on 05 June 2010 - 02:33 PM, said:

Doesn't software metric like cyclomatic complexity gives us the information on when and where to refactor?

I don't believe so.

The art of programing, the aesthetic choices a programmer makes when designing a program, are beyond the grasp of computer analysis. Given it's way, a computer can reduce human written code to an optimized set of instructions. Those instructions will be near impossible for a person to follow at a glance, but the computer will surely bless them as the best approach.

While a program is written to instruct a computer, it must also aid the programmer with it's design. It must break down into chunks that make sense to the observer ( us ), more than the consumer ( the infernal machine ). Because we are the ones the must maintain and further manipulate those instructions. So it's of greater importance that it's logically approachable, and apparently "simple", to us than to any algorithmic complexity test.