Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

Given a long method with Pac-Man ifs, would it be wise to:

  1. Break down code blocks into regions.
  2. Then refactor code blocks into methods.

Or would it be best to leave it alone?

Would it be risky?
Would it be a waste of time?

In the absence of automated unit-tests, I'm trying to understand the risk-reward relationship associated with this manuever.

share|improve this question
    
I don't get the "Pac-Man if", I googled and couldn't find the phrase used anywhere other than here. How did you come up with the name? –  Carson63000 Nov 12 '10 at 0:59
    
@Carson63000: That's why I made 'Pac-Man ifs' a hyperlink. –  Jim G. Dec 8 '10 at 3:34

3 Answers 3

up vote 1 down vote accepted

Your plan, or clean up the logic on the ifs (in cases where the pacman structure is an inefficient order or view of the conditions) is usually the place to start IMHO.

As to if it is worth the effort:

  • do you expect to be changing this page much?
  • do you have trouble understanding what is going on in there to the point you are wary of modifying it?
  • do you think it is causing a performance or stability problem?
  • is there no real reason it was done this way in the first place?
  • would it be trivial to manually validate the correctness of the re-factor?

IMHO unless you have some strong "yes" answers to those questions re-factoring it is a questionable exercise when compared to the risk of botching it and the fact that you could be adding new value elsewhere with your time.

share|improve this answer

It's risky in the absence of automated tests. Of course you'll test as you go along, but that's tedious, and error-prone. Ideally you'd write unit tests capturing the current behaviour, but that's not always feasible.

Having been in this position before (with PHP), if the code really stinks, refactor it very slowly and very carefully.

I think your plan's sound - take the long method, break it up with whitespace into paragraphs, and try Extract Method. Quite how you do that depends on how many variables are in play, and how they interact. Try trace the dependencies of a paragraph before you start.

Is it worth it? If the code's really awful, and its awfulness is starting to spread, then it's time to get stuck in there. If it's been running fine for ages, then you risk introducing bugs into something that for now Just Works.

share|improve this answer

0 . Work out what the logic of the if statements is.

Quite often when I'm looking at a set of pac-man ifs I find that if I just draw out something like a truth table of all the conditions involved I can work out a much better route to resolving the problem.

That way you can also assess whether there is a better method, how you might break it down further and ( and this is a big one with this kind of code ) whether there are any holes in the logic.

Having done that you can probably break it down into a couple of switch statements and a couple of methods and save the next poor mook who has to go through the code a whole lot of problems.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.