How to transition from bad code to good code.

Not for 'how-to' coding questions but PHP theory instead, this forum is here for those of us who wish to learn about design aspects of programming with PHP.

Moderator: General Moderators

jamesmm
Forum Newbie
Posts: 2
Joined: Wed Oct 17, 2007 4:09 am

How to transition from bad code to good code.

Post by jamesmm »

Hi All,
I've recently started work at a new company. There are about 5 developers ranging in skill level, and I have about 10 years general programming experience, with 5+ years PHP and OO design.

The current code base on our application is not in a good state. It was inherited from a single 'programmer' who produced it with no forethought or knowledge of programming or design good practice.

The company is expanding, and is healthy in terms of resources. I work for the technical manager, who is a good guy, I think open to suggestion and good ideas.

The issue at hand is that the code base is bad, and I don't know how much thought has been put into improving it by the current team or by my manager. It appears that at least in recent history all effort is going on to adding new features, which involves hacking the current bad code and little effort is put into spending time on structure improvement instead of new features. The code is so bad that there is no development environment and producing one isn't possible because of masses of hard coded URLs. I want to make sure I know what the best way to move ahead is so I can be constructive in terms of my work and suggestions, even if there is pressure to continue rolling out feature additions to the bad code.

I guess I'm looking for all creative comments about this situation - particularly: how have people successfully made the transition from a large bad application to a good new one (and from a large bad functional app to a large good OO app)? And how have people who have been in this situation where there's pressure to roll out new features into a bad code set responded?

Thanks for all info :)

j
alex.barylski
DevNet Evangelist
Posts: 6267
Joined: Tue Dec 21, 2004 5:00 pm
Location: Winnipeg

Post by alex.barylski »

Hard to say exactly without seeing the code. I would gander a guess and suggest it was likely developed either using multiple scripts or a single index.php script which handled all requests.

First thing I usually do (cause it doesn't really require a framework) is start refactoring out model code. Then Extract HTML into a template engine.

Look for duplicate code and begin thinking about how to centralize that code via a front controller or base page controller.

Start reading books on design patterns - it will help at least familiarize yourself with already known concepts and give you and your team a common vocabulary.

When you get stuck, drop by here and I am sure someone will assist you.

Bets of luck. :)
User avatar
CoderGoblin
DevNet Resident
Posts: 1425
Joined: Tue Mar 16, 2004 10:03 am
Location: Aachen, Germany

Post by CoderGoblin »

Really have 1 of 4 choices...

1) Total rewrite - Nighmare to do and test properly.
2) Rewrite as processed - when you need to use a piece of code rewrite it neatly.
3) Rewrite when required - only rewrite when code is required to be modified.
4) Ignore the problem

In reality a lot depends on the state of play with things like size of code/timescales/number of users of the existing code. One of the hardest problems is understanding the precise requirements of a piece of code, especially if it has "hidden side effects" or is poorly documented.
alex.barylski
DevNet Evangelist
Posts: 6267
Joined: Tue Dec 21, 2004 5:00 pm
Location: Winnipeg

Post by alex.barylski »

CoderGoblin wrote:Really have 1 of 4 choices...

1) Total rewrite - Nighmare to do and test properly.
2) Rewrite as processed - when you need to use a piece of code rewrite it neatly.
3) Rewrite when required - only rewrite when code is required to be modified.
4) Ignore the problem

In reality a lot depends on the state of play with things like size of code/timescales/number of users of the existing code. One of the hardest problems is understanding the precise requirements of a piece of code, especially if it has "hidden side effects" or is poorly documented.
Total Rewrite??? Refactoring isn't about totally rewritting it's about making small changes over time which lead up to large changes all the while attempting not to break your code while doing so.

It can be done, I just finished spending the last couple months refactoring a horrible codebase. Not exactly perfect but light years more organized than the original.
User avatar
Jenk
DevNet Master
Posts: 3587
Joined: Mon Sep 19, 2005 6:24 am
Location: London

Post by Jenk »

There's a good book on this very subject - title "Working Effectively with Legacy Code" published by Feathers. (Michael C. Feathers)
User avatar
CoderGoblin
DevNet Resident
Posts: 1425
Joined: Tue Mar 16, 2004 10:03 am
Location: Aachen, Germany

Post by CoderGoblin »

Hockey wrote: Total Rewrite??? Refactoring isn't about totally rewritting it's about making small changes over time which lead up to large changes all the while attempting not to break your code while doing so.
Total Rewrite is always an option, if not an option I would recommend. "Refactoring" as you call it is covered by points 2 and 3. The main point as we have both mentioned is not breaking things (although in a round about way by me).
User avatar
Maugrim_The_Reaper
DevNet Master
Posts: 2704
Joined: Tue Nov 02, 2004 5:43 am
Location: Ireland

Post by Maugrim_The_Reaper »

Rewriting is probably too big a job to visit.

I'd agree with Hockey on seeing how possible it is to extract Templating and Models/DatabaseAbstraction. Largely because from experience, legacy code in PHP is completely stuffed with major security flaws. All the hacking makes this even more likely. By extracting those two areas you can start enforcing SQL escaping and HTML output escaping centrally where it's harder for any one programmer to interfere with when making changes.

The other side is input filtering - sometimes you can try using ArrayAccess to implement a proxy layer between the raw $_GET/$_POST and the ones used by the application. You'd need to automate rules as much as possible to prevent editing code in too many places to safely pull off (esp. since you're no doubt without a testing suite to play safety net).

In all - I'd guess security is big weakness in what you have.
jamesmm
Forum Newbie
Posts: 2
Joined: Wed Oct 17, 2007 4:09 am

Post by jamesmm »

Interestingly, total rewrite is the main option that's in play at the moment. The code is a huge, huge mess, and I don't discount this as being the best option. It's not primarily a web application, although web pages are a part of it.

If rewriting from scratch is the option that's chosen, I wonder how long it will take while trying to maintain/add features to the existing code.

Anyone have any resources or thoughts on the process and planning of rewriting a bad functional application into a good OO one?

Thanks.
User avatar
Jenk
DevNet Master
Posts: 3587
Joined: Mon Sep 19, 2005 6:24 am
Location: London

Post by Jenk »

Use your old code as a "design spec" and create test cases for that criteria. Then produce code to pass your tests.

Voila. Rewrite done.*


*Easier said than done, but is still a very effective method.
User avatar
RobertGonzalez
Site Administrator
Posts: 14293
Joined: Tue Sep 09, 2003 6:04 pm
Location: Fremont, CA, USA

Post by RobertGonzalez »

I have been in the same boat. When I came to my company a year ago we had four different production web sites, four different development web sites and about five other SUDS (servers under desks). Everything was a mash of PHP 3 and PHP 4 code hitting literally hundreds of MySQL databases on many different boxes. Nothing was standard and nothing was central. All was crap.

The first thing I did was implement a web development standard that covered PHP code, HTML code, CSS code and Javascript code. Then I implemented a standard workflow process that included code reviews, source code control and a development push requirement prior to any code being pushed into production. We also began the process of segmenting all of our sites into their own separate machines over three layers: Dev, Test and Prod. We also created a server to house our source code.

We are probably 25% of the way through getting onto this new standard. Our new sites are developed in PHP E_ALL | E_STRICT compliant code with (X)HTML STRICT compliance. Period. Old code is maintained, but is being ported to new code to fit with the new PHP5 application framework I have built for the company. All direct queries are being pulled out of the code and are being replaced with stored procedures that either I or our DBA writes (but our DBA always reviews).

It is taking time, but it is converting from bad to better. Is this something that you could apply to your situation?
User avatar
Christopher
Site Administrator
Posts: 13596
Joined: Wed Aug 25, 2004 7:54 pm
Location: New York, NY, US

Post by Christopher »

As a practical matter you will probably rewrite the whole thing no matter which direction you go. That's really your goal. Though you will probably also keep much of algorithm code. Since rewriting it is inevitable, there is not much to decide. ;)

The harder question is where do you want to go. What will the code of the new codebase look like? Make those design decisions first, then go back and define a path from A -> B. I may not be the same for different parts of the code. So it is important identify how the current code is modularized.

I tend to tackle the easiest part first to learn and make mistakes on. I try rewrite in sections and build glue code to keep the whole system running during the transition. I also tend to choose to do less and do multiple passes, because I learn about the design of both the old and new code as I go.
(#10850)
User avatar
Kieran Huggins
DevNet Master
Posts: 3635
Joined: Wed Dec 06, 2006 4:14 pm
Location: Toronto, Canada
Contact:

Post by Kieran Huggins »

I'd have to vote for a total rewrite. There's no sense in re-writing the same code multiple times to interface with several iterations of the system.
start
I hope you'll be surprised by how quickly you can re-write an existing app, especially if you can lift the good algorithmic code and graft it in to the final product.

As with any new project, start by modeling your data. The rest comes rather naturally on top of a good model layer.

Maybe assign 3 or 4 people to servicing the existing codebase (and adding new features), and develop the replacement in parallel with the remaining team members.

Also, this would be a good opportunity to build upon a good MVC framework, which has the added benefit of enforcing best practices while compartmentalizing future code.
User avatar
CoderGoblin
DevNet Resident
Posts: 1425
Joined: Tue Mar 16, 2004 10:03 am
Location: Aachen, Germany

Post by CoderGoblin »

Everah wrote:The first thing I did was implement a web development standard that covered PHP code, HTML code, CSS code and Javascript code. Then I implemented a standard workflow process that included code reviews, source code control and a development push requirement prior to any code being pushed into production.
Agree with this 100% including the necessity of code reviews... The main difficulty you have is for everybody to "buy into" the program.
User avatar
RobertGonzalez
Site Administrator
Posts: 14293
Joined: Tue Sep 09, 2003 6:04 pm
Location: Fremont, CA, USA

Post by RobertGonzalez »

In our case, it wasn't hard. I proposed the changes to my IT Director before implementation. After it was implemented, I rolled it out to the company. It was a little challenging with the IT department as many of the users in our department were used to being mavericks (develop in production, code on the server, etc). But once we all bought in, everyone else fell into place.
User avatar
Jenk
DevNet Master
Posts: 3587
Joined: Mon Sep 19, 2005 6:24 am
Location: London

Post by Jenk »

I don't want to sound high and mighty with this, but I've now typed out 3 different responses, taking nearly 10mins each, with each sounding as bad as the previous, so apologies..

If you are using strong test cases, which you and your colleagues have reviewed, you often find code reviews become uneccessary in most cases. Complex functionality or areas of functionality that take time to run should be reviewed to ensure it is efficient, but if the test case runs quick and passes, there is no point reviewing the code - the test case is reviewing it for you.

The key to this is that you *must* review your test cases, with a rigid process - i.e. cases cannot be "accepted" until formally reviewed or such like. They really are the key-stone of a project. They are also the first point of call for all changes - bugs, change requests etc. Change the test case to suit the requirements, have it reviewed, then all you have to do is refactor your code to pass the test. In fact it's often preferable for someone else, who is unaware of the required change to do the refactoring so that they are not influenced or biased by the change - rarely happens though, just an ideology. Because of this, it's sometimes a lot less time consuming to go ahead and start from scratch and focus only on passing the tests - forget the rest of the project/application even exists; providing you pass the test case, it'll slot right in.
Last edited by Jenk on Wed Oct 17, 2007 7:05 pm, edited 1 time in total.
Post Reply