Topic: refactoring - how to start?

i have received a rails app (or rather a rails app has recieved me.. )

These are the salient features -
23 controllers.
2 routes : map.connect '',:controller =>'login', :action => 'login', map.connect ':controller/:action/:id'
each controller has 2000+ lines of code.
every ajax request is mapped to its own controller action.
100+ actions/controller.
view logic in controller.
business logic in view.
view code in layout files.
2000+ loc per model.
empty helpers.

!!No unit tests/rspec.

Where and how do I start?

Re: refactoring - how to start?

1) Go to a high building if you aren't already inside one.
2) Find a way onto the roof.
3) Jump.

Ok, seriously, that sounds horrible, you have my condolences.

What is you goal with this application, what are you expected to do? Make it faster? make it REST? only clean it up?
That would be important facts to determine a route for this project.

Anyways, i would probably start like this:

1) clean up helpers
2) clean up layouts & views, move logic into  models(!!) and/or controllers & helpers
3) that should make the views less clumsy, DRY things up ...
4) now that views and controllers are thinner, you can think about implementing REST and DRYing things up even more  -depending on what exactly your final goal here is, as i asked above.

Re: refactoring - how to start?

Oh dear!! How did you end up with this? Is this for a client or do you work for a company.

I think it is very important that you make it clear to stake holders that whatever they want you to do with this app is fraught with danger due to the condition it is in. Someone else's crap has become your problem, don't let it become your fault as well!!

Re: refactoring - how to start?

hey duplex,
  Thanks for your suggestions, I was wondering in this case  if 'a grand rewrite' module by module would be easier.
these are my reasons:

1. even the naming convention within these classes are pretty bad. Its genuinely hard to figure out what each function does. Eg. in a report'ing'_controller there is a function add_new_column(count,report) and another add_report_column and another function add_column.

2. The database has been denormalized to address performance problems due to badly designed databases (incremental design mess) and bad sql queries.

Id be interested in your reasons for the rewrite or against the rewrite?
p.s.  I like the way you say 'only' clean it up.

@cherring, I joined a product company as an employee for whom this is the sole product. It is a well known problem - but the people in the drivers seat are the good business folk. My understanding is they are not worried about or cannot connect IT risks (which I see now as a delivery risk rather than a productivity risk) to business delivery risks.

Re: refactoring - how to start?

Well really you need to ask yourself "Will it take me longer to start over, or re work the current code?". Obviously to re write it, you need to make sure you know all of the required features. With knowing those features, do up an ERD or a Class diagram to get a better idea of what models youll require.

Im not saying to go that route necessarily, but youll want to weigh the cost/benefits of going either direction.

A project I did in school involved PHP. While we kept it object oriented, it at the same time wasnt very dry. We had classes for the models, and one for storing the database information, but that was about it. So when at the end the client and I figured out the things to come for this project (at the end of the final semester), I decided to try putting it into rails. I managed to get some basics up really quickly, so ive been working at it since. The program now has sooo much more control with admin capability, and it also is so much neater and more manageable. Theres still some features to implement, and others to finalize and tidy up. Other than that Im happy with my decision on this. FYI the client is another student, the business plan was a group project for him, but now its more entrepreneurial.

Re: refactoring - how to start?

Here's my tuppence worth having been dumped into this situation myself many times previously.
I would refactor as I go.

The overriding factor is to tackle what is relevant and only what is relevant.

You need to start small, and think small.
Take one view or report at a time and deal with just that one small area.
This approach stops you getting carried away and means that you get to understand that area and what it is doing (or supposed to be doing) really quickly. It means you get work delivered on time and to spec and in budget and you get to rewrite the whole app little by little without needing to understand the whole application in one big chunk

So how to refactor that one view
This is what I would do
Determine what that view's purpose is, what it updates and what it's business logic is.
Once you understand that you can write a test (and a spec if you use cucumber or the likes).
This is REALLY important as it helps to test your full knowledge of that view as well as ensures that the code does what it's supposed to do.

Now you have a test (and spec?) you can start refactoring.
set up a restfull route, move a tiny bit of business logic into the model, add a new table view that represents the data as it should be or write a finder or named scope. whatever it takes to get just that one view working the way it should work, passing all tests and not interfering with any of the other parts of the app.

Once you have a whole controller, model and views refactored with using views in the database rather than tables you can consider creating a new table the way it should be and switch from using views to using the table.

Obviously refactoring the database is going to be a bit tricky and that approach may not be practical but once you have the tests in place it will become easier to do the refactoring of the database with confidence that you will know if you break something pretty quickly.

What I'm trying to say is that it is far easier and more effective to become familiar with small sections than it is to become familiar with all the quirks of a large app and you will no doubt be being asked to produce new code and releases pretty quickly so those areas that need the attention are probably the best to start with.

hope that helps

What you want and what you need are too often not the same thing!
When your head is hurting from trying to solve a problem, stop standing on it. When you are the right way up you will see the problem differently and you just might find the solution.
(Quote by me 15th July 2009)

Re: refactoring - how to start?

jamesw is right, you can't safely refactor this unless you write some tests first - otherwise you can't see if you've broken anything. One approach is to take a 20,000 feet view - is there a lot of duplication that has a common theme e.g. are all your controllers mostly cut & pastes of each other, with a few minor differences? If so you could factor out the common stuff.

Start small, and start with the tests. History is littered with the remains of good applications founded on bad code, that fell by the wayside because the maintainers couldn't keep up.

Re: refactoring - how to start?

jamesw is right, you can't safely refactor this unless you write some tests first - otherwise you can't see if you've broken anything. One approach is to take a 20,000 feet view - is there a lot of duplication that has a common theme e.g. are all your controllers mostly cut & pastes of each other, with a few minor differences? If so you could factor out the common stuff.

Start small, and start with the tests. History is littered with the remains of good applications founded on bad code, that fell by the wayside because the maintainers couldn't keep up.

Re: refactoring - how to start?

thanks everyone this has been very helpful. refactor it is.

Re: refactoring - how to start?

one bit of useful advice with adding testing is to an existing app is to start as broad as possible, then get more detailed as you go along - for this reason, selenium is good as it's acceptance testing, ie it simulates a user using the application, rather than testing the code. 

I imagine that as you start working on it all of your early work will be on the low-level stuff like db schemas and models.  A suite of selenium tests will tell you when any of these randomly break some of the pages, which sounds like it will be the case a lot for you.  So, you can beaver away under the surface, making things nice, but know when you've broken the basic usability of the app.

I'm not saying you shouldn't use TDD as well, but this is coming at it from the opposite end to TDD, just a basic "does it still work" test.

Last edited by Max Williams (2009-10-22 10:54:55)

###########################################
#If i've helped you then please recommend me at Working With Rails:
#http://www.workingwithrails.com/person/ … i-williams

Re: refactoring - how to start?

Max Williams wrote:

one bit of useful advice with adding testing is to an existing app is to start as broad as possible, then get more detailed as you go along - for this reason, selenium is good as it's acceptance testing, ie it simulates a user using the application, rather than testing the code. 

I imagine that as you start working on it all of your early work will be on the low-level stuff like db schemas and models.  A suite of selenium tests will tell you when any of these randomly break some of the pages, which sounds like it will be the case a lot for you.  So, you can beaver away under the surface, making things nice, but know when you've broken the basic usability of the app.

I'm not saying you shouldn't use TDD as well, but this is coming at it from the opposite end to TDD, just a basic "does it still work" test.

oh and you have my sympathies btw, that sounds like a fucking nightmare sad

###########################################
#If i've helped you then please recommend me at Working With Rails:
#http://www.workingwithrails.com/person/ … i-williams

Re: refactoring - how to start?

yup ... thats a saving grace ... there are selenium tests already.

Re: refactoring - how to start?

spacefugitive wrote:

yup ... thats a saving grace ... there are selenium tests already.

haha, well yeah that's something, although by the sounds of it i wouldn't necessarily trust them to cover the site properly.

###########################################
#If i've helped you then please recommend me at Working With Rails:
#http://www.workingwithrails.com/person/ … i-williams