• We’re currently investigating an issue related to the forum theme and styling that is impacting page layout and visual formatting. The problem has been identified, and we are actively working on a resolution. There is no impact to user data or functionality, this is strictly a front-end display issue. We’ll post an update once the fix has been deployed. Thanks for your patience while we get this sorted.

refactoring spaghetti code into good code

IHateMyJob2004

IHateMyJob2004

Lifer
I have the task of refactoring 3 years of Java development by a junior developer (pretty much not mentored) into more elegant code. The good news is that the junior developer knows that this refactoring is needed. It's not his fault that things got where they are.

We want to use good structures and design patterns. We have an idea of how to do this but I was wondering if there are any defacto standard books to read or articles or web sites or anything. I want to see if there are any less obvious things we need to consider.

Our general plan is to refactor code areas that are not getting updated as new features are added. To an extent this is possible but at some point it won't be. We know this.

I don't want to discuss the project details here. I am mostly looking for books and sources of information on best strategies.
 
My biggest piece of advice, make sure you have test coverage before you start any refactoring.

Refactoring one piece of spaghetti code WILL break other parts of your codebase, parts that you didn't think are even related sometimes.
 
Crusty said:
My biggest piece of advice, make sure you have test coverage before you start any refactoring.

Refactoring one piece of spaghetti code WILL break other parts of your codebase, parts that you didn't think are even related sometimes.
Click to expand...

Agreed, this and just take it bit by bit, and have a good process for feeding the test results back into development.
 
Hahahhaa, you think the code has unit tests. How quaint.

Remember, code that is not designed is not easy/possible to test. We are dealing with APIs that make unit testing almost impossible because there is now proxy layer to strip out APIs. It's a catch 22.
 
Crusty said:
My biggest piece of advice, make sure you have test coverage before you start any refactoring.

Refactoring one piece of spaghetti code WILL break other parts of your codebase, parts that you didn't think are even related sometimes.
Click to expand...

Yup. Even if the tests aren't good enough to stay long term, having them there short term to ensure the functionality doesn't break helps a lot.

For example. I had one method that did WAY too much. It was one of those 500 line behemoths. Before approaching it, I wrote tests against that method which exercised every code path. After that, refactoring was a breeze. I was able to rewrite the whole thing. The test wasn't a waste of time, it caught several issues during the course of the rewrite that would have been hard to detect.

Though, after that was done, I deleted those tests. Why? Because they were way too deep and would have broken too frequently. (I had many different tests for the broken up methods instead that where testing much more shallow, so I wasn't losing anything by removing it).
 
IHateMyJob2004 said:
Hahahhaa, you think the code has unit tests. How quaint.

Remember, code that is not designed is not easy/possible to test. We are dealing with APIs that make unit testing almost impossible because there is now proxy layer to strip out APIs. It's a catch 22.
Click to expand...

Even if the tests don't stay long term (they themselves would be a maintenance task), writing tests that do deep integration sorts of checks is pretty useful when you are refactoring.
 
IHateMyJob2004 said:
Hahahhaa, you think the code has unit tests. How quaint.

Remember, code that is not designed is not easy/possible to test. We are dealing with APIs that make unit testing almost impossible because there is now proxy layer to strip out APIs. It's a catch 22.
Click to expand...

Not sure what language you are using, but lots of testing facilities have the ability to mock objects for you. With a little bit of effort you should be able to build a mock API to test against.
 
Oh, yes we intend to add unit tests. Probably during refactoring. But before might happen.

Not my first rodeo. I am pretty confident that I can refactor the code and not introduce new bugs (that are not obvious)
 
My advice would be to start with unit tests, rely on refactoring TOOLS as much as possible (i.e. eliminate human error), and every refactoring step should go toward a particular design goal. Small methods does not equal clean code, although clean code usually means small methods.

Focus on lowering cyclomatic complexity, aggressively promote immutability (use "final" as much as possible, eliminate setters as much as possible), and eliminate variations of the wheel (that is, if a framework or library exists that does what part of your code is doing, throw it away and use the library). That aside, I would say the last thing I'd look at is the actual hierarchical structure of the classes. Once code is clean, it's much easier to collapse into reusable code and all that OOP goodness.

Good luck 🙂
 
definitely write the unit tests before refactoring, otherwise you really will have no clue whether or not you broke something while doing so.
 
purbeast0 said:
definitely write the unit tests before refactoring, otherwise you really will have no clue whether or not you broke something while doing so.
Click to expand...


Seconded.

without the unit tests, you will have no real idea if things are actually working except for the end result. One failure, and you will have no idea where to look.

Also, doing the tests will provide some template for the rework.
 
slugg said:
My advice would be to start with unit tests, rely on refactoring TOOLS as much as possible (i.e. eliminate human error), and every refactoring step should go toward a particular design goal. Small methods does not equal clean code, although clean code usually means small methods.

Focus on lowering cyclomatic complexity, aggressively promote immutability (use "final" as much as possible, eliminate setters as much as possible), and eliminate variations of the wheel (that is, if a framework or library exists that does what part of your code is doing, throw it away and use the library). That aside, I would say the last thing I'd look at is the actual hierarchical structure of the classes. Once code is clean, it's much easier to collapse into reusable code and all that OOP goodness.

Good luck 🙂
Click to expand...

Wow, the first use of cyclomatic complexity that I agree with. Ever.

Wheel ... oh .. reinventing the wheel. Ya, I found one instance of that. Also, the original dev didn't know about the vecmath libraries when he started. I should let you know that we are doing customization on a 3D cad tool.

Good comments.
 
You must log in or register to reply here.

TRENDING THREADS

Back
Top