search

h4kuna / fio

Read and send payment order for FIO bank, PSR-17 ready
54 stars 25 forks source link

Refactoring: mad FioRead and FioPay stateless #19

Closed foxycode closed 8 years ago

foxycode commented 8 years ago

I think these two classes shoul be stateless. The will be much more simplier to use and testable this ways. This is only first look how I think. If you like it, I will continue polishing it.

foxycode commented 8 years ago

Ten poslední český komentář zmizel po mém pushi, tak to píšu sem. To že se Bank stará o validaci účtů chápu, ale co se týče plateb do zahraničí, tka moc k ničemu není. Neodliší totiž české číslo účtu od IBANu. Pokud to chceš mít takto striktní, tak bych ud2lal objekty pro jednotlivé typy (CZAccount, IBANAccount např.) a ty by pak jednotlivé typy plateb mohly p5ijímat v konstruktoru. Co ty na to?

h4kuna commented 8 years ago

Ano Bank momentálně v tomhle je k ničemu a popravdě iban jen schovává účet v sobě.

Jenom myšlenky:

Validace IBANu nikde není momentálně.

foxycode commented 8 years ago

Mě se dost líbí jak to mají vyřešené tady: https://github.com/sunfoxcz/spayd-php Hlavně tohle: https://github.com/sunfoxcz/spayd-php/blob/master/src/Shoptet/Spayd/Model/CzechAccount.php A tohle: https://github.com/sunfoxcz/spayd-php/blob/master/src/Shoptet/Spayd/Utilities/IbanUtilities.php

Tak bychom se mohli inspirovat :)

foxycode commented 8 years ago

BTW: Tahle chyba s XML se mi zobrazovala ještě před mými úpravami a nevím co s ní :( https://travis-ci.org/h4kuna/fio/jobs/102047559#L188

foxycode commented 8 years ago

Teď ještě koukám že AccountCollection::accountExists je private. Říkal jsem si, že to označování active úplně vyhodím a nechám tam jen že první přidaný účet bude default. Bude to tak ok?

h4kuna commented 8 years ago

Nemám problém, když to mají přes composer nahradit Bank tímto https://github.com/Shoptet/spayd-php a používat to. Maj i slovenký účty což je taky potřeba.

h4kuna commented 8 years ago

Na tu chybu s xml musím kouknout a nějak ji nasimulovat.

h4kuna commented 8 years ago

První přidaný byl i předtím default. Jen teď se nepřepínaj takže active postrádá smysl.

h4kuna commented 8 years ago

Testy u tebe na stroji fungují?

foxycode commented 8 years ago

U mě fungovaly až na ten XML, ten druhý probl0m se u mě neprojevoval, ale já tedy ještě zlikviduju ty active a tím to opravím.

foxycode commented 8 years ago

@h4kuna Je tam můj update. Nevíš co s tím XML errorem? Nemůžu to dohledat.

h4kuna commented 8 years ago

Teď už se tomu nemohu věnovat, kouknu na to zase večer :)

h4kuna commented 8 years ago

@foxycode Test je opraven v posledním komitu.

foxycode commented 8 years ago

Snad je to takto ve stavu, který ut Ti bude vyhovovat.

h4kuna commented 8 years ago

Nice work.

foxycode commented 8 years ago

Thanks for merging. Do you plan to merge it to master soon? Can I try to integrate spayd-php?

h4kuna commented 8 years ago

I fix fio-nette and merge to master. After you will integrate spayd-php, I release stable.