2011-02-09

TDD utflykt i PHP land, del 4

Förra inlägget avslutades med att lite "fusk" för att hantera koordinaten. Nu ska det bli ändring på det. Efter att ha funderat lite så har jag bestämt mig för att prova att återanvända den presentatör och visare jag redan gjort för koordinat. Skillnaden blir att själva HTML:en görs som ett tillägg till Smarty.

Första steget blir att modifiera namnen på variablerna i Request-objektet. Därefter borde det vara möjligt för ChildWp_Presenter att skapa en Coordinate_Presenter och delegera till den. Det modifierade testfallet ser ut så här.

function testChildWpIsAdded() { $request = new Http_Request(); $childWpHandler = new MockChildWp_Handler(); $request->set('wp_type', 1); $request->set(Coordinate_Presenter::lat_hem, 'N'); $request->set(Coordinate_Presenter::lat_deg, '10'); $request->set(Coordinate_Presenter::lat_min, '15'); $request->set(Coordinate_Presenter::lon_hem, 'E'); $request->set(Coordinate_Presenter::lon_deg, '20'); $request->set(Coordinate_Presenter::lon_min, '30'); $request->set('desc', 'my waypoint'); $childWpHandler->expectOnce('add', array(1, 10.25, 20.5, 'my waypoint')); $presenter = new ChildWp_Presenter($request); $presenter->addWaypoint($childWpHandler); }

Coordinate_Presenter skapas i konstruktorn. Förändringen i addWaypoint() blir liten.

public function addWaypoint($childWpHandler) { $coordinate = $this->coordinate->getCoordinate(); $childWpHandler->add($this->getType(), $coordinate->latitude(), $coordinate->longitude(), $this->getDesc()); }

Att få in koordinaten var enkelt. Däremot är det fortfarande mer komplicerat att sätta upp förutsättningarna i testfallet. Istället för att ange de enskilda komponenterna i en koordinat så hade jag hellre gjort det på ett mer abstrakt sätt. Det här är typiskt en sådan sak man skriver upp på att-göra-listan, men låter vara för tillfället.

Det känns som det blir lättast att skippa visaren helt och istället låta presentatören sätta värdena direkt med assign(). Om det hade funnits några krav på kompatibilitet så hade jag skapat en ny visare vars uppgift skulle vara att översätta anropen till motsvarande variabelvärden (Adapter pattern). Men nu finns inte det så därför tar jag tillfället i akt och gör mig av med kod.

Det är en sak som testfallet ovan saknar och det är cachen som punkten är knuten till. För att kunna lägga till en ny punkt måste man veta vilken cache den tillhör. Det verkar rimligt att anta att cachens ID är angivet i URL:en.

function testChildWpIsAdded() { $request = new Http_Request(); $childWpHandler = new MockChildWp_Handler(); $request->set('cacheid', 2); $request->set('wp_type', 1); $request->set(Coordinate_Presenter::lat_hem, 'N'); $request->set(Coordinate_Presenter::lat_deg, '10'); $request->set(Coordinate_Presenter::lat_min, '15'); $request->set(Coordinate_Presenter::lon_hem, 'E'); $request->set(Coordinate_Presenter::lon_deg, '20'); $request->set(Coordinate_Presenter::lon_min, '30'); $request->set('desc', 'my waypoint'); $childWpHandler->expectOnce('add', array(2, 1, 10.25, 20.5, 'my waypoint')); $presenter = new ChildWp_Presenter($request); $presenter->addWaypoint($childWpHandler); }

Vad händer om cachens ID saknas? Det är uppenbarligen ett fel och ett lämpligt felmeddelande ska visas istället. Det finns redan färdiga felkoder och mallen har en speciell metod för fel, så testet blir förhållandevis enkelt.

function testSetsErrorIfNoCacheId() { $presenter = new ChildWp_Presenter(); $presenter->init($this); $this->assertEqual(ERROR_CACHE_NOT_EXISTS, $this->errorCode); }

Som vanligt vet jag inte riktigt hur gränssnittet ska se ut. Tanken med init() är att den kontrollerar förutsättningarna och anropar error() på mallobjektet om det är något fel som gör att sidan inte kan visas. I del 2 introducerade jag validate() som anropades när sidan postades tillbaka. Jag vet inte om det samma sak här eller inte, men det känns inte så. Namnet skulle kunna vara bättre också.

Jag tänker gå ifrån min klass för mallobjekt och istället låta test-klassen implementera samma gränssnitt som mallen. Det känns inte som att en särskild klass för mallen tillför något.

För att kunna lägga till en punkt till en cache så måste cachen finnas. Så nästa steg blir att utföra den kontrollen. Cache är ett centralt begrepp och det ligger nära till hands att anta det kommer behövas en klass för cache. Men istället för att skapa något som kanske kan vara bra att ha någon gång i framtiden, så tänker jag göra minsta möjliga för att lösa det aktuella problemet.

Det enklaste är om man kan fråga någon om det finns en cache med ett visst ID. Det här med namn är svårt, så det får bli en managerare tills vidare. Det verkar enklast att skicka in den som argument i init(), så jag gör det till att börja med.

Det blir två testfall. Ett för att verifiera att rätt felkod sätts om den inte finns en cache med angivet ID. Och ett som verifierar att inget fel sätts om cachen finns. Tanken är att exists() själv får avgöra om ID:t har giltigt format.

function testSetsErrorIfCacheDoesNotExist() { $cacheManager = new MockCache_Manager(); $_GET['cacheid'] = '234'; $cacheManager->setReturnValue('exists', false); $cacheManager->expectOnce('exists', array('234')); $presenter = new ChildWp_Presenter(); $presenter->init($this, $cacheManager); $this->assertEqual(ERROR_CACHE_NOT_EXISTS, $this->errorCode); } function testDoesNotSetErrorIfCacheExists() { $cacheManager = new MockCache_Manager(); $_GET['cacheid'] = '345'; $cacheManager->setReturnValue('exists', true); $cacheManager->expectOnce('exists', array('345')); $presenter = new ChildWp_Presenter(); $presenter->init($this, $cacheManager); $this->assertEqual(0, $this->errorCode); }

Jag är inte helt säker på hur detta bör implementeras. Första problemet är att komma åt 'cacheid' från Request-objektet. Man kommer bara åt värden som har blivit validerade, så vidare man inte använder getForValidation(). Det känns inte som presentatörens uppgift att validera 'cacheid'. Den ska inte veta vad som är ett giltigt cache-ID. Det hela kompliceras ytterligare av den nya metoden init() som (än så länge) görs innan den ordinarie valideringen (när formuläret postas tillbaka) är tänkt att göras. Det hela kanske klarnar när valideringen och tillbaka postningen kommer på plats.

public function init($template, $cacheManager) { $cacheid = $this->request->getForValidation('cacheid'); if (!$cacheManager->exists($cacheid)) $template->error(ERROR_CACHE_NOT_EXISTS); }

Ett annat fall som ska generera ett fel är om en användare försöker lägga till en punkt till en cache som denna inte äger. Användare är också ett grundläggande begrepp, så det känns inte aktuellt att göra något mer ingående än att överlämna ansvaret till CacheManager. Det centrala är att presentatörer kontrollerar att användaren får lägga till en punkt. Jag använder medvetet samma felkod även om cachen faktiskt finns.

function testSetsErrorIfUserMayNotModifyCache() { $cacheManager = new MockCache_Manager(); $_GET['cacheid'] = '345'; $cacheManager->setReturnValue('exists', true); $cacheManager->expectOnce('exists', array('345')); $cacheManager->setReturnValue('userMayModify', false); $cacheManager->expectOnce('userMayModify', array('345')); $presenter = new ChildWp_Presenter(); $presenter->init($this, $cacheManager); $this->assertEqual(ERROR_CACHE_NOT_EXISTS, $this->errorCode); }

Återigen är gränssnittet för CacheManager lite trevande. Kanske skulle man kunna slå ihop de två metoderna och låta userMayModify() kontrollera existensen också? Men jag föredrar att ta det beslutet när det är dags att faktiskt implementera CacheManager.

Nackdelen med svagt typade språk är att en förändring i gränssnittet inte märks förrän koden som är beroende av gränssnittet exekveras. Men genom att använda en mock av objektet så kommer testfallen att fallera om gränssnittet ändras.

public function init($template, $cacheManager) { $cacheid = $this->request->getForValidation('cacheid'); if (!$cacheManager->exists($cacheid) || !$cacheManager->userMayModify($cacheid)) $template->error(ERROR_CACHE_NOT_EXISTS); }

Då har vi kommit så pass långt att vi kontrollerar att cachen existerar och att användaren också får göra tillägg till cachen. Sidan fylls i med standardvärden. (Nästan i alla fall, fortfarande så saknas typen på punkten.) Vidare kan en punkt skapas baserat på informationen i fälten. Dock måste vi validera värdena i fälten innan vi kan använda dem.

Det känns som att det inte går att skjuta upp typen på punkten längre, så det får bli nästa sak att ta hand om.

Inga kommentarer: