living with legacy code
DESCRIPTION
Practical tips for dealing with projects involving legacy code. Covers investigating past projects, static analysis of existing code, and methods for changing legacy code. Presented at PHP Benelux '10TRANSCRIPT
Living
with
Legacy Code
@rowan_m
@rowan_m
Ibuildings
Plusnet
What is“Legacy Code”?
Code without tests
Code you've “inherited”
Code no-one understands
Technical debt
What is“Legacy Code”?
Who hasnever created
Legacy Code?
My own
story
little
My own
story
little
Pragmatic, not idealistic
Starting a project
Aim to understandthe concepts
and motivations
Try usingthe application
Find and then readany / all
documentation
Checklist
Official:Project brief ( )Requirements ( )Tech. Spec. ( )
Actual:Emails ( )Meeting notes ( )
Progress:Gant charts ( )Burndowns ( )Overtime logs ( )
Quality:Bug tracker ( )Complaints ( )User Forums ( )
Talk to theoriginal developers
Talk to the users
… especially the“different” ones
Approachingthe code
Catalogue thelive platform& environment
Recreate it!
Deploy the code
Checklist
PHP:php.ini ( )PEAR / PECL modules ( )Compile options ( )Patches ( )
The Rest:OS ( )Package manager ( )Web server ( )Web server modules ( )Site config. ( )Database ( )Cache ( )JS libraries ( )Firewall rules ( )Proxies ( )Services running ( )
Time to enterThe Code
Reading
Static analysis
Dynamic analysis
Time to enterThe Code
phpdoc
phpdoc -ti 'Sweet Application' \ -pp -o HTML:Smarty:PHP \ -d Libraries \ -t Docs
http://www.phpdoc.org/
Title
Style
Code in here
Docs out here!
Beware of type-hiding!
Type-hinting
Type-hiding
/** * @param array $opts Current options * @return array Options with flag set */function setFlag(array $opts) { $opts['flag'] = true; return $opts;}
/** * @param int $fullPence Full price in pence * @return float Discounted price in pence */function applyDiscount($fullPence) { return ($fullPence * 0.8);}
doxygen
http://www.stack.nl/~dimitri/doxygen/
doxygen -s -g ~/doxy.confvim ~/doxy.conf
# edit at least thisOUPUT_DIRECTORY# play with the rest
cd ~/dev/projectdoxygen ~/doxy.conf
Docs out here
Code in here
ctags
#!/bin/bashcd ~/Dev/ &&ctags-exuberant -f ~/.vimtags \-h ".php" -R \--exclude="\.git" \--links=no \--totals=yes \--tag-relative=yes \--PHP-kinds=+cf \--regex-PHP='/abstract\s+class\s+([^ ]+)/\1/c/' \--regex-PHP='/interface\s+([^ ]+)/\1/c/' \--regex-PHP='/(public\s+|static\s+|abstract\s+|protected\s+|private\s+) ↵ function\s+\&?\s*([^ (]+)/\2/f/'
Code in here
Tags out here
Voodoo
http://ctags.sourceforge.net/
bouml http://www.bouml.fr/
bouml http://bouml.free.fr/
bouml http://bouml.free.fr/
bouml http://bouml.free.fr/
bouml http://bouml.free.fr/
bouml http://bouml.free.fr/
bouml
codesniffer
rowan@swordbean:~/Dev/ZendFramework-1.9.4/library/Zend/Service$ phpcs \ --standard=Zend Exception.php
FILE: /home/rowan/Dev/ZendFramework-1.9.4/library/Zend/Service/Exception.php--------------------------------------------------------------------------------FOUND 1 ERROR(S) AND 2 WARNING(S) AFFECTING 3 LINE(S)-------------------------------------------------------------------------------- 17 | WARNING | Line exceeds 80 characters; contains 87 characters 32 | WARNING | Line exceeds 80 characters; contains 87 characters 36 | ERROR | Opening class brace must be on a line by itself 36 | ERROR | Closing brace must be on a line by itself--------------------------------------------------------------------------------
http://pear.php.net/package/PHP_CodeSniffer/
Continuo
us
Integrat
ion
http://jenkins-ci.org/
http://phpundercontrol.org/
http://sismo.sensiolabs.org/
Decision time!
Decision time!
Ignore it,code anyway
Rewrite,
refactor
,
test, tes
t, test
Ignore it,code anyway
Ignore it,code anyway
Please don't.
Ignore it,code anyway
Please don't.
however...
Deadlines, clients,money, etc.
Deadlines, clients,money, etc.
Secure afollow-up project
Make everyoneaware of the risks
Why do you need the code?
Why do you need the code?
Library dependency
Adding new behaviour
Changing behaviour
simple
complex
Isolatelegacy dependencies
Isolatelegacy dependencies
Create ananti-corruption layer
Complete isolation
Create alegacy service
Partial isolation
Wrapper classesor methods
<?phpinclude('/home/victorvon/secrets.inc');
/** * @param array $person willing volunteer */function extract_brain(&$person) { $brain = $person['brain']; unset($person['brain']);
return $brain;}
/** * @param array $person * @return bool living or not */function create_life($person) { require_once(LIB_DIR.'../nuts_n_bolts.inc'); kerzap(); $person['living'] = true; return $person;}?>
Some codeyou need
:(
Wrapper class
class VictorWrapper{
public function __construct() {
require_once '/home/victorvon/tragedy.php';}
public function extractBrain(Person $p) {// format to legacy style$pLgcy = $this->toArray($p);// run legacy code$bLgy = extract_brain($pLgcy);// format to new style$p = $this->toPerson($pLgcy);$b = $this->toBrain($bLgcy);return array($p, $b);
}
public function createLife(Person $p) {
// validateif ($person->isAlive()) throw new LivingException();// format to legacy style$pLgcy = $this->toArray($p);// run legacy code$pLgy = create_life($pLgcy);// format to new stylereturn $this->toPerson($pLgcy);
}}
Some codeyou can use
:)
Wrapper class
Changing the code
Changing the code
Take anincremental approach
Commit to 1 dayat a time
Why?
Difficult to estimate
Hidden dependencies
Unknown behaviour
1. Get it intoversion control
2. Identify theinflection point
3. Create integration& acceptance tests
4. Set up yourcontinuous integration
environment
5. Rewrite and refactor!
Types
of
changes
Mixed Procedural→
includes
HTML PHP
PHP
HTML
HTML
PHP
HTML
HTML
HTML
PHP
PHPPHP
HTML
PHP
PHP HTML
HTML
includes
HTML PHP
PHP
HTML
HTML
PHP
HTML
HTML
HTML
PHP
PHPPHP
HTML
includes
HTML echo
foreach
foreach
HTML
HTML
HTML
echo
ifif
HTML
PHP
PHP HTML
HTML
HTML
HTML echo
echo HTML
HTML
function
function
function
Procedural OO→
includes
function
function
function
free code
function
includes
static method
static method
static method
static method
static method
includes
function
function
function
free code
function
includes
static method
static method
static method
static method
static method
includes
method
method
method
method
constructor
includes
function
function
function
free code
function
Sprout method / class
public function createInvoice(Account $acc, array $charges){ $invoice = new Invoice();
foreach ($charges as $chg) { $invoice->addLine($chg->getDesc(), $chg->getAmount()); }
return $invoice;}
The existing code
“We just need to be able to give each client their own personal discount on certain charges.”
public function createInvoice(Account $acc, array $charges){ $invoice = new Invoice();
foreach ($charges as $chg) { $invoice->addLine($chg->getDesc(), $chg->getAmount()); }
return $invoice;}
private function calcDiscount(Account $acc, Charge $chg){ $accDisc = new AccountDiscounter($acc);
$discountedCharge = $accDisc->calculate($chg); return $discountedCharge;}
The new code
public function createInvoice(Account $acc, array $charges){ $invoice = new Invoice();
foreach ($charges as $chg) {
// Sprout new behaviour!$chg = $this->calcDiscount($acc, $chg);
$invoice->addLine($chg->getDesc(), $chg->getAmount()); }
return $invoice;}
private function calcDiscount(Account $acc, Charge $chg){ $accDisc = new AccountDiscounter($acc);
$discountedCharge = $accDisc->calculate($chg); return $discountedCharge;}
Call it
Untestable OO testable OO→
Dependency Inversion / Extraction
The Problempublic function calcDiscount(Account $acc, Charge $chg){ $accDisc = new AccountDiscounter($acc);
$discountedCharge = $accDisc->calculate($chg); return $discountedCharge;} Untestable!
The Problempublic function calcDiscount(Account $acc, Charge $chg){ $accDisc = new AccountDiscounter($acc);
$discountedCharge = $accDisc->calculate($chg); return $discountedCharge;} Untestable!
class AccountDiscounter{ public function __construct(Account $acc) { // check cache // contact the database // call a web service }}
Quick Solutionpublic function calcDiscount(Account $acc, Charge $chg){ $accDisc = $this->getAccountDiscounter($acc);
$discountedCharge = $accDisc->calculate($chg); return $discountedCharge;}
protected function getAccountDiscounter(Account $acc){ return new AccountDiscounter($acc);}
Mock object in your testOverride method
Dependency Injection Solution
public function calcDiscount(Account $acc, Charge $chg){ $accDisc = $this->discounter;
$discountedCharge = $accDisc->calculate($chg); return $discountedCharge;}
public function __construct(AccountDiscounter $ad){ $this->discounter = $ad;}
Pass it into the class
(v2) Dependency Injection Solution→
public function calcDiscount(Account $acc, Charge $chg){ $accDisc = $this->discounter;
$discountedCharge = $accDisc->calculate($chg); return $discountedCharge;}
public function __construct(IAccountDiscounter $ad){ $this->discounter = $ad;}
Make an interface
Analyse
Plan
Test
IsolatE
Change
Test MORE
Summary
Any questions?
Feedback to:
https://joind.in/6020
@rowan_m
Credits
http://www.flickr.com/photos/flatlinevision/1514971535/http://commons.wikimedia.org/wiki/File:Weird_Tales_November_1950.jpghttp://commons.wikimedia.org/wiki/File:AdventuresIntoDarkness1401.jpghttp://www.flickr.com/photos/locationscout/3594432797/http://www.flickr.com/photos/rawhead/3466304669/http://commons.wikimedia.org/wiki/File:Rocket_to_the_Moon_54893.jpghttp://commons.wikimedia.org/wiki/File:Weird_Chills_July.jpghttp://commons.wikimedia.org/wiki/File:Terrific_01.jpghttp://commons.wikimedia.org/wiki/File:Strange_Fantasy_01.jpghttp://commons.wikimedia.org/wiki/File:Beware_01.JPGhttp://www.flickr.com/photos/erokcom/2873449983/http://www.flickr.com/photos/locationscout/3594433235/http://commons.wikimedia.org/wiki/File:Weird_Chills_Sept.JPGhttp://commons.wikimedia.org/wiki/File:Weird_Comics_01.JPGhttp://www.flickr.com/photos/x-ray_delta_one/3972988193/http://commons.wikimedia.org/wiki/File:Weird_Tales_January_1950.jpghttp://commons.wikimedia.org/wiki/File:Plan_nine_from_outer_space.jpghttp://www.flickr.com/photos/javyer/3545217741/http://www.flickr.com/photos/76074333@N00/318034222/http://commons.wikimedia.org/wiki/File:Dime_Mystery_Magazine_July_1934.jpghttp://www.flickr.com/photos/quinnanya/3802177022/
Further reading:Michael FeathersMartin Fowler