Skillnaden mellan att skapa en ny punkt och redigera en befintlig punkt är minimal. I alla fall ur användarperspektiv. Implementationsmässigt skiljer det sig en del.
Första frågan är, hur vet vi om det handlar om att lägga till en ny punkt eller redigera en befintlig? Svaret är att den inkommande URL:en skiljer sig åt. När vi redigerar en punkt talar vi om vilken det gäller genom att ange dess ID. I teorin räcker det med att ange punktens ID, men eftersom vi ändå behöver cachens ID (vi måste fortfarande verifiera att användaren får modifiera punkten) så känns det enklast om även cachens ID kommer via URL:en.
Att verifiera att punkten faktiskt tillhör cachen känns som ett enkelt testfall. Men ett ännu enklare testfall är att verifiera att felsidan visas om punkten inte finns.
function testSetsErrorIfWaypointDoesNotExist()
{
$cacheManager = new MockCache_Manager();
$childWpHandler = new MockChildWp_Handler();
$this->request->setForValidation(ChildWp_Presenter::req_cache_id, '345');
$this->request->setForValidation(ChildWp_Presenter::req_child_id, '456');
$cacheManager->setReturnValue('exists', true);
$cacheManager->expectOnce('exists', array('345'));
$cacheManager->setReturnValue('userMayModify', true);
$cacheManager->expectOnce('userMayModify', array('345'));
$childWpHandler->expectOnce('exists', array('456'));
$childWpHandler->setReturnValue('exists', false);
$presenter = $this->createPresenter();
$presenter->init($this, $cacheManager, $childWpHandler);
$this->assertEqual(ERROR_CACHE_NOT_EXISTS, $this->errorCode);
}
Det första som slår mig är att det behövs mycket kod för att sätta upp testfallet. Men innan jag kan göra något åt det så måste init() modifieras så att testfallet blir godkänt.
public function init($template, $cacheManager, $childWpHandler = false)
{
$cacheid = $this->request->getForValidation(self::req_cache_id);
if (!$cacheManager->exists($cacheid) || !$cacheManager->userMayModify($cacheid))
$template->error(ERROR_CACHE_NOT_EXISTS);
else
$this->request->validate(self::req_cache_id, new Validator_AlwaysValid());
$childid = $this->request->getForValidation(self::req_child_id);
if (!empty($childid) && !$childWpHandler->exists($childid))
$template->error(ERROR_CACHE_NOT_EXISTS);
}
Vinsten med att verifiera att Cache_Manager faktiskt blir anropad är marginell. Det som är av intresse är hur presentatören hanterar de olika returvärdena. Så därför skapar jag en Cache_Manager som alltid initieras med ett par fasta ID:n.
function setUp()
{
...
$this->cacheManager = new MockCache_Manager();
$this->cacheManager->setReturnValue('exists', true, array('234'));
$this->cacheManager->setReturnValue('exists', true, array('345'));
$this->cacheManager->setReturnValue('exists', false);
$this->cacheManager->setReturnValue('userMayModify', true, array('345'));
$this->cacheManager->setReturnValue('userMayModify', false);
}
Det senaste testfallet blir nu lite enklare.
function testSetsErrorIfWaypointDoesNotExist()
{
$childWpHandler = new MockChildWp_Handler();
$this->request->setForValidation(ChildWp_Presenter::req_cache_id, '345');
$this->request->setForValidation(ChildWp_Presenter::req_child_id, '456');
$childWpHandler->expectOnce('exists', array('456'));
$childWpHandler->setReturnValue('exists', false);
$presenter = $this->createPresenter();
$presenter->init($this, $cacheManager, $childWpHandler);
$this->assertEqual(ERROR_CACHE_NOT_EXISTS, $this->errorCode);
}
Nästa test blir att verifiera att felsidan visas om punkten inte tillhör den angivna cachen.
function testSetsErrorIfWaypointDoesNotBelongToCache()
{
$childWpHandler = new MockChildWp_Handler();
$this->request->setForValidation(ChildWp_Presenter::req_cache_id, '345');
$this->request->setForValidation(ChildWp_Presenter::req_child_id, '567');
$childWpHandler->setReturnValue('exists', true, array('567'));
$childWpHandler->expectOnce('getCacheId', array('567'));
$childWpHandler->setReturnValue('getCacheId', 234);
$presenter = $this->createPresenter();
$presenter->init($this, $this->cacheManager, $childWpHandler);
$this->assertEqual(ERROR_CACHE_NOT_EXISTS, $this->errorCode);
}
(Dessutom blir det ett liknande testfall som verifierar att felsidan inte visas om punkten tillhör cachen.)
Nästa testfall handlar det om att verifiera att användargränssnittet blir korrekt ifyllt med värdena från den befintliga punkten. Men hur får man tag på värdena? En variant är ett objekt som har lämpliga get()-metoder för att komma åt respektive värde. En enklare variant är en vektor med alla värden. Jag börjar med den enklare varianten.
function testExistingWaypointIsShownAfterPrepare()
{
$childWpHandler = new MockChildWp_Handler();
$this->request->setForValidation(ChildWp_Presenter::req_cache_id, '345');
$this->request->setForValidation(ChildWp_Presenter::req_child_id, '567');
$childWpHandler->setReturnValue('exists', true, array('567'));
$childWpHandler->setReturnValue('getCacheId', 345, array('567'));
$childWpHandler->setReturnValue('getChildWp', array('type' => '3', 'latitude' => 20.5, 'longitude' => 30.75,
'description' => 'Start here.'), array('567'));
$presenter = $this->createPresenter();
$presenter->init($this, $this->cacheManager, $childWpHandler);
$presenter->prepare($this);
$this->assertEqual('3', $this->values[ChildWp_Presenter::tpl_wp_type]);
$this->assertEqual('N', $this->values[Coordinate_Presenter::lat_hem]);
$this->assertEqual(20, $this->values[Coordinate_Presenter::lat_deg]);
$this->assertEqual(30, $this->values[Coordinate_Presenter::lat_min]);
$this->assertEqual('E', $this->values[Coordinate_Presenter::lon_hem]);
$this->assertEqual(30, $this->values[Coordinate_Presenter::lon_deg]);
$this->assertEqual(45, $this->values[Coordinate_Presenter::lon_min]);
$this->assertEqual('Start here.', $this->values[ChildWp_Presenter::tpl_wp_desc]);
}
Onekligen mycket kod i testfallet. Det blir även hel del kod i init().
public function init($template, $cacheManager, $childWpHandler = false)
{
$cacheid = $this->request->getForValidation(self::req_cache_id);
if (!$cacheManager->exists($cacheid) || !$cacheManager->userMayModify($cacheid))
$template->error(ERROR_CACHE_NOT_EXISTS);
else
$this->request->validate(self::req_cache_id, new Validator_AlwaysValid());
$childid = $this->request->getForValidation(self::req_child_id);
if (!empty($childid))
{
if (!$childWpHandler->exists($childid) || $cacheid != $childWpHandler->getCacheId($childid))
$template->error(ERROR_CACHE_NOT_EXISTS);
$childWp = $childWpHandler->getChildWp($childid);
$this->type = $childWp['type'];
$this->description = $childWp['description'];
$this->coordinate->init($childWp['latitude'], $childWp['longitude']);
}
}
private function getType()
{
return $this->request->get(self::req_wp_type, $this->type);
}
private function getDesc()
{
return $this->request->get(self::req_wp_desc, $this->description);
}
Det finns en del att förbättra. Testfallet kan göras lite enklare genom en särskild verifieringsmetod för koordinaten. Dessutom vill jag ändra lite på gränssnittet till ChildWp_Handler. Om man kommer åt cachens ID via vektorn som returneras av getChildWp() så skulle man kunna skippa både exists() och getCacheId().
Testfallet efter ändringarna.
function testExistingWaypointIsShownAfterPrepare()
{
$childWpHandler = new MockChildWp_Handler();
$this->request->setForValidation(ChildWp_Presenter::req_cache_id, '345');
$this->request->setForValidation(ChildWp_Presenter::req_child_id, '567');
$childWpHandler->setReturnValue('getChildWp', array('cacheid' => '345', 'type' => '3', 'latitude' => 20.5,
'longitude' => 30.75, 'description' => 'Start here.'), array('567'));
$presenter = $this->createPresenter();
$presenter->init($this, $this->cacheManager, $childWpHandler);
$presenter->prepare($this);
$this->assertEqual('3', $this->values[ChildWp_Presenter::tpl_wp_type]);
$this->assertCoordinate('N', 20, 30, 'E', 30, 45);
$this->assertEqual('Start here.', $this->values[ChildWp_Presenter::tpl_wp_desc]);
}
Och init().
public function init($template, $cacheManager, $childWpHandler = false)
{
...
$childid = $this->request->getForValidation(self::req_child_id);
if (!empty($childid))
{
$childWp = $childWpHandler->getChildWp($childid);
if (empty($childWp) || $cacheid != $childWp['cacheid'])
$template->error(ERROR_CACHE_NOT_EXISTS);
$this->type = $childWp['type'];
$this->description = $childWp['description'];
$this->coordinate->init($childWp['latitude'], $childWp['longitude']);
}
}
De hårdkodade strängarna borde man göra något åt, men jag vet inte riktig vad än.
Härnäst väljer jag att verifiera att punkten blir uppdaterad när den har redigerats. Detta görs på liknande sätt som för skapandet av punkt, dvs verifiera att ChildWp_Handler blir anropad med rätt parametrar.
function testChildWpIsUpdated()
{
$this->request->set(ChildWp_Presenter::req_child_id, 3);
$this->request->set(ChildWp_Presenter::req_wp_type, 1);
$this->request->set(Coordinate_Presenter::lat_hem, 'N');
$this->request->set(Coordinate_Presenter::lat_deg, '10');
$this->request->set(Coordinate_Presenter::lat_min, '15');
$this->request->set(Coordinate_Presenter::lon_hem, 'E');
$this->request->set(Coordinate_Presenter::lon_deg, '20');
$this->request->set(Coordinate_Presenter::lon_min, '30');
$this->request->set(ChildWp_Presenter::req_wp_desc, 'my waypoint');
$childWpHandler = new MockChildWp_Handler();
$childWpHandler->expectOnce('update', array(3, 1, 10.25, 20.5, 'my waypoint'));
$presenter = $this->createPresenter();
$presenter->updateWaypoint($childWpHandler);
}
Koden blir nästan identisk med addWaypoint().
public function updateWaypoint($childWpHandler)
{
$coordinate = $this->coordinate->getCoordinate();
$description = htmlspecialchars($this->getDesc(), ENT_COMPAT, 'UTF-8');
$childWpHandler->update($this->getChildId(), $this->getType(), $coordinate->latitude(),
$coordinate->longitude(), $description);
}
Det finns ytterligare två saker att ta skriva testfall för. Titeln på sidan ska vara annorlunda när man redigerar en punkt. Dessutom måste punktens ID postas tillbaka, annars går det inte att uppdatera den.
Testfallet är väldigt likt det föregående så det ligger nära till hands att införa vissa förenklingar. Precis som för Cache_Manager skapas en ChildWp_Handler i setUp() tillsammans med ett par lämpliga returvärden.
function testPageTitleEditIsTranslated()
{
$this->request->setForValidation(ChildWp_Presenter::req_cache_id, '345');
$this->request->setForValidation(ChildWp_Presenter::req_child_id, '567');
$presenter = $this->createPresenter();
$presenter->init($this, $this->cacheManager, $this->childWpHandler);
$presenter->prepare($this);
$this->assertEqual('Edit waypoint tr', $this->values[ChildWp_Presenter::tpl_page_title]);
}
Det enda vi behöver för att lösa problemet är en variabel som kan berätta om det handlar om skapa ny punkt eller redigera befintlig. Jag väljer att ta punktens ID som sätts i init(). Då behöver man inte validera den för att komma åt den senare, utan kan använda den direkt.
public function prepare($template)
{
if ($this->childid)
$template->assign(self::tpl_page_title, $this->translator->Translate('Edit waypoint'));
else
$template->assign(self::tpl_page_title, $this->translator->Translate('Add waypoint'));
...
}
Implementationen av addWaypoint() och updateWaypoint() är väldigt lika. Dessutom känns det inte helt naturligt att koden i php-sidan ska hålla reda på om det är en ny punkt som ska läggas till eller om det är en befintlig som ska uppdateras. Därför slås de ihop till en metod kallad doSubmit(). Med tanke på att ChildWp_Handler redan skickas in i init() så känns det onödigt att göra det ytterligare en gång.
På ett par ställen i presentatören används en if-sats för att hantera skillnaden mellan lägg till ny punkt och redigera befintligt punkt. Jag tycker inte om det, och hade hellre använt Strategy pattern, men det är tolerabelt*.
Avslutningsvis koden för php-sidan.
<?php
require('common.inc.php');
$tpl->name = 'childwp';
$isSubmit = isset($_POST['submitform']);
$redirect = isset($_POST['back']);
$request = new Http_Request();
$presenter = new ChildWp_Presenter($request);
$cacheManager = new Cache_Manager();
$handler = new ChildWp_Handler();
$presenter->init($tpl, $cacheManager, $handler);
$presenter->setTypes(array(new ChildWp_Type(1, 'Parking'), new ChildWp_Type(2, 'Reference point')));
if ($isSubmit && $presenter->validate())
{
$presenter->doSubmit();
$redirect = true;
}
if ($redirect)
$tpl->redirect('editcache.php?cacheid=' . $presenter->getCacheId());
$presenter->prepare($tpl);
$tpl->display();
?>
Härmed är utflykten slut för denna gång.
*) Senare, när ta bort punkt implementerades, valde jag att applicera Strategy pattern och dela upp presentatören i tre olika varianter med en gemensam basklass. Dessutom bröt jag ut koden i init() och la i en egen klass istället.
Inga kommentarer:
Skicka en kommentar