What is legacy? Code you didn't write Code you wouldn't write Untested code Code written by people with conflicting visions What do we do with it? Refactor: _safely_ changing your code without changing the behavior Bad code smells No separation between PHP and HTML Lots of require's, few method calls Global variables Separating controllers and views Even without a solid MVC architecture, this helps Doable in several safe and easy steps Will find pain points Why do I need to do this? Code complexity will increase echo isn't as fun as it looks You will find hidden bugs and mistakes Obvious improvements to make error handling assignment by reference changing view path use-specific subclasses The separation process gather all your code sift and separate controller from view code assign variables to the view object change all variable references in the view code split the files find duplicated views The rules of view code Allowed Control structures echo, or display-specific functions, never nested Not allowed assignment other function calls Gather and sift code: the part you won't like the step you won't like: gather all code for this controller draw a line at the top of the code move controller code above this line, fixing as necessary at this point, everything is view code Alternative control structures Barely documented in PHP Helpful for non-PHP web developers ... ... A frustrating problem: display data in a loop Don't want to do it earlier and loop twice Can create a new array of variables for your view Can encapsulate your variables in an object (*better*) $order->getId(), $order->getStatus(), etc Change all variables to view object variables assign vars to view object $view->assign('foo', $foo); one by one, change vars in view code test to convince yourself iterate document inputs to the view Separate the files create new file for view code important! search and replace $view with $this test Find duplicated views Re-work repetition to re-use view files Include views in other views with $this->render('included_file.tpl'); Untangling a require web require statements that call other require statements can get very complex dependent on application structure Why untangle? remove unneeded complexity create less procedural code prior to 5.2, require_once and include_once are expensive if you are requiring class defs, and you have a std file naming method, use __autoload() The untangling process ID inputs ID outputs wrap file in a method refactor method move method to correct location ID inputs and outputs find all vars expected to be set before this file is included 1 way: Execute this file by itself find all vars expected to be set or mutated by this file set vars are easy: comment out the require and watch errors mutated is the set of inputs changed. learn to search for these! wrap file in a function wrap entire include in a func pass all input vars return all output vars as an array then call that func at the bottom of the required file! this is a mess! Refactor until complete tease out funcs or objects inside this func if returning lots fo data, see if it can be an object leave temp big func in place so outside code doesn't break. keep updating it to deal with your refactoring Hide SQL code in a separate function because it's a change in state: PHP to SQL Move to correct location figure out where these funcs or objs should live in your app move them find where the require is called through your app, and replace that with your new func call or obj method Global variables everywhere $_POST and $_GET session/cookie data database handles user account language Do you still have register_globals on Bad idea. Possible to fix. Turn on E_ALL (all uninitialized vars throw a warning) Spider your site and grep for uninitialized vars Not as much work as you think. It's worth it. $_POST and $_GET These aren't horrible But not horrible isn't a very high standard Having an inputvariable class works. __construct(), isSet(), isGet(), isPost(), getAsString(), getAsInt() Database global object Very common in PHP Not horrible but bad Prevents testing DB changes, so having a state to test from is hard Prevents multiple databases Parameterizing the DB handle does it need to be everywhere? can you pass it in to a function or constructor? process is simple Add DB param pass in that global var if call not in global scope, find out how to pass in that var to the current scope Maybe it does have to be everywhere use a singleton but not really make a way to change the singleton instance global define or env var static mutator (his choice) Further reading Working effectively with legacy code, Michael Feathers more approachable, even better than Fowler book Refactoring, Martin Fowler (classic in the field)