Třída pro práci s Webkalkulačkou Cetelemu

Tomáš Jacík

Zdravím,

dal jsem včera dokupy třídu pro práci s Cetelemem. Info je zde: https://github.com/…oxcz/cetelem

Je to můj první takovýto počin, tak bych ocenil nějaké tipy co upravit/vylepšit. Hlavně co do struktury kódu a testovatelnosti. Dříve jsem takto s testy nic nedělal. Je to prozatím koncipováno pro použití v Nette, ale používá to jen některé komponenty, tak to teoreticky půjde použít i jinde.

David Matějka

Tak rychle jsem to projel a:

pri predavani objektu je zbytecny pouzivat &
snaz se pouzivat anglictinu
rozdel CetelemUver na request a response
nejlepe odeber public properties a pouzij gettery/settery
nepouzivej Kdyby\Curl\CurlWrapper, ale Kdyby\Curl\CurlSender. Vytvoris Kdyby\Curl\Request, posles do „send“ a vrati se ti Kdyby\Curl\Response
nenastavuj annotation parseru cache, tady by ses o to starat nemel
pokud nedostanes cache storage, pouzij fallback na memory storage – pak se nebudes muset starat, jestli cache existuje nebo ne
pro ziskani reflexe muzes pouzit Nette\Reflection\ClassType::from (posles tam objekt)
pouzivej slozene zavorky i u jednoradkovych if-u

Editoval matej21 (23. 6. 2014 16:20)

Tomáš Jacík

Díky za tipy, ale budu k tomu mít pár dotazů:

Mám za to, že při běžném předávání do funkce vzniká kopie (i když asi nějaká weak o instancí). Ten objekt uvnitř funkce modifikuju, potřebuju, aby se ty modifikace dostaly ven a přišlo mi zbytečný aby se vytvářely kopie. Nebo se v PHP něco změnilo a funguje to teď jinak?
Vím, ten mix EN a CZ je tam divný, ještě to chci poladit, nicméně Cetelem sám je česká služba a chtěl bych zachovat názvy, které používají. Nechci to tedy ohýbat přes koleny a vymýšlet názvy jako getInsurance nebo getInstallment atd. Pokud máš nějaké konkrétnější tipy na nšikovné použití češtiny tak za ně budu rád, ale dělat to celé v EN si myslím postrádá smysl.
To API Cetelemu je takové dost nešikovné, můžeš posílat jen některé parametry + další nepovinné a ty ovlivňují vše ostatní, včetně těch, které jsi poslal. Tedy i Tvé vlastní vstupy se vracejí jinak. Proto jem zvolil objekt, kde si můžeš nadefinovat jen to co potřebuješ a zbytek se doplní/upraví z API. Proto jsem zvolil jeden objekt, který se předává do funkce odkazem a tedy není žádný return. Pokud má to oddělení nějaký hlubší smysl, tak prosím o osvětlení.
K čemu to bude dobré? Jediné co mě napadá, tak zajistit, aby některé z těch properties byly read-only, ale musím je nějak nasetovat z API. Máš nějaký tip?
CurlWrapper jsem použil jen pro lepší testovatelnost. K čemu tam dělat instance dvou dalších objektů, když to jde takto jendoduše? Má to nějakou výhodu oproti mému přístupu?
Asi chápu kam míříš, tohle by mělo nastavovat Nette, že? Co ale v případech, kdy tu libku chceš použít samostatně v něčem jiném, jak se s tím vypořádat? Tu cache je dobré tam mít.
To je dobrý nápad, díky. Zase to ale trochu vadí v případě, když tu libku budu chtít využít bez Nette. Nyní je tam ta Nette cache povinná v balíku, tak to asi dává smysl, nebylo by ale lepší, když to půjde použít i úplně bez cache?
To jsem přehlédl, díky.
To bych také mohl upravit.

David Matějka

nepamatuju si, ze by php nekdy klonovalo objekty pri predavani.
radeji getInsurance nez getPojisteni :)
aha, ok
to souvisi se zapouzdrenim objektu v OOP, nemel bys mit moznost pristupovat k vnitrnim properties primo, ale pouze pres verejne API, ktere zajisti konzistenci dat apod.
imho je curlwrapper spise internal helper pro kdyby/curl. CurlSender poskytuje hezci API a komunikace tam probiha formou request – response. To, ze se vytvareji dve nove instance vubec neres – dulezitejsi je hezky kod
tak bys mel napsat do dokumentace „pokud nepouzivate nette blabla“. Tvoje extension by nemelo takhle ovlivnovat globalni prostredi
pokud bys tam chtel mit moznost bez nette/cache, tak bych vytvoril nejaky cache adapter a nette cache by byla jen jedna z implementaci. Pak by to slo napojit i na uplne jinou cache.

Editoval matej21 (23. 6. 2014 17:32)

Filip Procházka

1. to tak fungovalo v PHP 4, ale (minimálně) od 5.2 už se objekty vždy předávají referencí.

5. CurlWrapper má stav, pokud ho chceš používat, tak bys správně měl vždy vytvořit novou instanci, né si ho předávat konstruktorem. CurlSender nemá stav, jen výchozí nastavení, proto je vhodný jako služba. Dalším problémem je, že CurlWrapper je jenom objektová obálka na curl resourcem, CurlSender + jeho request/response ti řeší spoustu edge casů v komunikaci pomocí curl a háže výjimky.

Tomáš Jacík

Tak jsem si tím přepisem na CurlSender zkomplikoval testování :(

Nyní to mám takto:

<?php

$request = new Kdyby\Curl\Request($url);
$response = $this->curl->send($request);
$xml = $response->getResponse();

?>

Nemohu ale v mockistovi otestovat, že jde o Request:

<?php

$request = new \Kdyby\Curl\Request('https://www.cetelem.cz:8654/bareminfo.php?kodProdejce=2044576');

$builder = $mockista->createBuilder('\Kdyby\Curl\CurlSender');
$builder->send($request)->once();

?>

Ty Request objekty prostě nejsou stejné a v mockistovi jsem nenašel nic, co by testovalo jen zda jde o instanci té samé třídy:

<?php

Mockista\MockException: Unexpected call in mock \Kdyby\Curl\CurlSender::send()

?>

Nějaké rady?

Editoval foxycode (24. 6. 2014 16:41)

Filip Procházka

A proč testuješ že ti přijde konkrétní instance, když máš testovat že ti přijde objekt nějakého typu s nějakými hodnotami? Mockista neumí equals místo same?

Tomáš Jacík

Pokud umí, tak jsem to tam nenašel. I když to zadávám takto

<?php
$curlSenderMock->expects('send')->once()->andReturn($resultMock);
?>

tak když přidám volání with, je konec. Ze zdrojáku koukám, že používá spl_object_hash. Klidně ocením i tip na jiný mock nástroj, který to bude umět porovnat správně.

Tomáš Jacík

Mám ještě jeden problém, se kterým si nevím rady. Mám tento test:

<?php

$mockista = new \Mockista\Registry();

$builder = $mockista->createBuilder('\Kdyby\Curl\Response');
$builder->getResponse()->once()->andReturn(
	'<bareminfo><chyba>Error</chyba></bareminfo>'
);
$resultMock = $builder->getMock();

$builder = $mockista->createBuilder('\Kdyby\Curl\CurlSender');
$curlSenderMock = $builder->getMock();
$curlSenderMock->expects('send')->once()->andReturn($resultMock);

$cetelem = new Cetelem\Cetelem('2044576', $curlSenderMock);
$cetelem->setDebug(TRUE);

Assert::exception($cetelem->getBarem(), 'Cetelem\XMLResponseException', 'Error');

$mockista->assertExpectations();

?>

A vyhodí mi tuto chybu:

<?php

-- FAILED: Sunfox\Cetelem\Cetelem XMLResponseException test. | tests/Cetelem/Cetelem.XMLResponseException.phpt
   Exited with error code 255 (expected 0)

   Fatal error: Class 'Sunfox\Cetelem\XMLResponseException' not found in /home/liquid/projects/cetelem/src/Cetelem/Cetelem.php on line 89

?>

Nemám tušení, proč tu class nenajde, nejprve jsem ji měl v src/Cetelem/exceptions.php, ale zkoušel jsem ji přidat i do src/Cetelem/XMLResponseException.php. Namespace mám nastaven správně a stejně nic.

David Matějka

@foxycode: pouzivas i v testech autoloading composeru? Mas tam totiz classmap, ktery se autimaticky nerefreshne. Pokud tedy pridas tridu, musis

composer dump-autoload

Tomáš Jacík

@matej21 To bylo ono. Díky moc :)

Tomáš Jacík

Tak nyní to už classu s exception najde, ale pro změnu mám tuto chybu:

<?php
 Exited with error code 255 (expected 0)
   Sunfox\Cetelem\XMLResponseException: Error
?>

Dle všeho by to mělo fungovat pro

Assert::exception($cetelem->getBarem(), ‚Sunfox\Cetelem\XMLResponseException‘, ‚Error‘);

David Matějka

@foxycode musis to uzavrit do closury (resp. predavat callable)

Assert::exception(function () { $cetelem->getBarem(); }, 'Sunfox\Cetelem\XMLResponseException', 'Error');

Editoval matej21 (24. 6. 2014 20:13)

Tomáš Jacík

@matej21 Máš pravdu, zrovna teď jsem to načetl. Problém je, že tu proměnnou $cetelem vevnitř nevidí, global nefunguje a uses nefunguje na PHP 5.3 :( Jde to i jinak, nebo musím udělat upgrade PHP?

Editoval foxycode (24. 6. 2014 20:18)

David Matějka

@foxycode use funguje uz od 5.3. Ale melo by stacit predat callable:

Assert::exception(array($cetelem, 'getBarem'), ...)

Tomáš Jacík

Měl jsem tam špatně něco jiného, nakonec mi s tím use funguje :) Vše je venku na GITu.
Díky vám oběma za pomoc.

Filip Procházka

Assert::exception(function () use ($cetelem) {
    $cetelem->getBarem();
}, 'Sunfox\Cetelem\XMLResponseException', 'Error');