OOP PHP - Începător

Inițial ți-am sugerat cu clone. Și mie îmi surâde această idee, dar asta nu înseamnă că aș face așa.

Aia mai pragmatică din punctul meu de vedere e Map rămâne practic cum e, dar adaugi două noi interfețe pentru read-only și modifiable map, pe care Map le implementează. Lui Player îi dai doar contractul read-only. La runtime există un singur obiect Map pentru toată lumea, dar în IDE (autocomplete), player vede doar un map citibil.

Nu e soluția "perfectă" și e discutabil cum definim "perfect", dar din punctul meu de vedere e o balanță sănătoasă între over-engineering și "deschiderea de porți" pe viitor. Adăugarea celor două interfețe și a clauzei "Map implements..." durează 1 minut. Partea bună la această abordare e expresivitatea codului: specifică clar cine are drepturi de modificare ale celei mai importante componente: map.

După cum am scris, varianta cu clone îmi place, dar nu aș face-o. În ce situații aș face-o? Dacă într-un fel sau altul jucători ar putea uploada propriile strategii de AI, de exemplu. În general, trebuie să te uiți mai atent la cerințe (ce zice product owner că vrea).


Pe de o parte vrei să deschizi porți, pe de alta PO te apreciază dacă menții costurile sub control, deci nu vrei să faci overengineering.


Cum e mai bine să faci în fiecare situație înveți cu experiența, acum ne imaginăm doar ca să vezi câteva moduri de gândire.

Dincolo de acest proiect, cel mai bun sfat pe care ți-l pot da e să lucrezi cu clienți adevărați, să te pui în locul lor, să înțelegi ce vor de fapt (când chiar nici ei nu înțeleg ce vor) și să citești "pragmatic programmer".

Dacă nu ai clienți adevărați (printr-un angajator/firmă sau direct/PFA) atunci îți poți crea clienți adevărați prin open-source, trebuie doar să îi tratezi de ca și cum ar fi adevărați.

Open-source nu e o pierdere de timp, dacă faci măcar cât de cât ceva ce nu mai face nimeni altcineva, și poate deschide oportunități unde ai leverage în negocieri.

Gata,destul din "diverse", la commits cu tine.

OriginalCopy, on 31 august 2018 - 17:08, said:

Nu ma prind cum vine asta. Deci Map ramane o singura clasa si creez un singur obiect $map. Map implementeaza doua interfete, ReadOnlyMap si WritableMap. Apoi lui Player ii dau doar contractul read-only.

interface ReadOnlyMap {}
interface WritableMap {}
class Map implements ReadOnlyMap, WritableMap {}
class Player {
private $map;
public function __construct (ReadOnlyMap $map) {}
}
$map = new Map ();
$player = new Player ($map);

Atunci player are acces la intregul obiect $map, in ciuda 'restrictiei' din propriul constructor. Are si metodele cerute de WritableMap. Restrictia e doar pentru wolfenste incepatorul care se bazeaza pe autocomplete din IDE si care nu vede metodele de scriere asupra lui map prin intermediul lui player? Iar acest autocomplete face asta uitandu-se in constructorul lui player? Nu stiu d'astea, nu am folosit vreodata IDE. Imi trebuie un singur lucru neaparat: un editor de text avand background negru.

Edited by wolfenste, 01 September 2018 - 17:14.

Implicit nu le are (ar trebui sa faca urat runtime-ul/compilatorul), decat daca se apuca sa sape pe unde nu trebuie.

wolfenste, on 01 septembrie 2018 - 16:47, said:

Da, PHP, cu bunele și cu relele sale, nu e suficient de strict. Există scule care automatizează aceste verificări (de static code analysis, ex. PHPQA).

Într-un proiect real de tictactoe la acest nivel eu ca lead developer/whatever doar atât de departe aș merge, și în rest aș prinde neregulile în timpul code review.

Eu așa aș face, dar chiar nu ai nicio restricție aici și poți merge cu clone dacă asta te face să te simți mai în siguranță.


Important e să știi ce alte opțiuni există și de ce faci ceea ce faci, nu să iei decizii pe care nu le poți raționaliza.

Ai mână liberă.


Eu aș merge cu read vs write pentru expresivitatea codului, claritatea în intenție.

Ca o notă: de ani buni, php devine din ce în ce mai strict, deci nu m-ar mira dacă în viitor (sau chiar acum, nu am testat) php ar face verificările necesare.

Deocamdata nu verifica. Urmatorul cod se executa fara erori sau avertismente.

  1 <?php
  2
  3 interface ReadOnlyMap {
  4	 public function read_me ();
  5 }
  6 interface WritableMap {
  7	 public function write_me ();
  8 }
  9
10 class Map implements ReadOnlyMap, WritableMap {
11	 public function read_me () {}
12	 public function write_me () {}
13 }
14
15 class Player {
16	 public $map;
17	
18	 public function __construct (ReadOnlyMap $map) {
19		 $this->map = $map;
20	 }
21 }
22
23 $map = new Map ();
24 $player = new Player ($map);
25
26 $player->map->write_me ();

Codul care ar viola contractul ar fi $this->map->write_me() in interiorul lui Player.

Deasemenea, strict_types ar putea interfera.

Dar nu conteaza, am testat si nu face inca lucrurile astea.

Am intalnit si primul bug mai indaratnic. Testul tests/src/NextMoveProviderPlayerTest::player_made_an_unavailable_choice trece cu brio (asteapta o exceptie). Asta er trebui sa testeze linia 45 din src/NextMoveProviderPlayer. Si totusi CodeCoverage il pastreaza cu rosu. Btw, sunt pe branch-ul attempt2.

Si $this->map->isMapAvailable nu arunca o exceptie?

Urmareste pas cu pas code coverage, iti spune destul de exact ce se intampla.

Cum lucrezi cu exceptiile, in special daca sunt asa tranziente (bubbling up of exceptions) printre componente (o exceptie originara in Map care trece prin NextMoveProviderPlayer catre apelant) e un subiect mai complicat.

Bug-ul e probabil undeva in codul tau. Ruleaza toate testele, daca ai conceput o suita buna de teste, atunci ele iti vor spune unde e buba. Daca nu iti spun sau nu poti deduce destul de rapid, atunci ceva e gresit in structura testelor, sau teste lipsesc, sau sau sau.

Oricare ar fi greseala, invata din ea si repar-o. Si spune-ne si noua ce ai invatat.

Intuitia imi spune ca ar trebui sa adresezi mai intai problema asta https://github.com/w...d6fcd#r30395512

Jonglezi prea mult cu int <-> MapCoordinate. Foloseste MapCoordinate, si converteste la int doar chiar in locul unde trebuie sa faci o operatie matematica sau logica cu int.

Foloseste value objects cu incredere, te protejeaza de multe buguri si frustrari.

Edited by OriginalCopy, 05 September 2018 - 08:44.

Pentru ca umblu cu doua lucruri diferite care inseamna acelasi lucru. Pe de o parte index-ul din array-ul din Map inseamna o pozitie pe harta, pe de alta MapCoordinate tot aia inseamna Si trebuie apoi conversii si sincronizari. Dar las' ca indexul ala e bun cand o sa trec la calcule de determinare a castigatorului (pentru ca nu voi folosi metoda din branch-ul inspiration) si voi avea nevoie chiar de numerele 0 - 8.

Am inteles unde e problema din semnatura lui setMarksCell. Voi folosi pana in ultima clipa obiectul MapCoordinate care e deja validat.

LE: imi veni o idee nastrusnica. Pana la urma, index-ul din Map e o chestiune 'intima' a lui Map si nimeni (nici macar MapCoordinate) nu ar avea de ce sa stie de el. Peste tot e pasat MapCoordinate (chiar si cand se interogheaza disponibilitatea hartii) iar doar Map face conversia intern si raspunde interogarilor.

alt LE: Ca downside nu Map ci Game decide castigatorul si ar cam avea nevoie de un index. Deci ramane asa.

inca un LE: Am 'rezolvat' si bug-ul. Ghilimele pentru ca e ca atunci cand stergi un director intreg stiind ca e un virus acolo fara sa stii exact unde e si ce fel de virus e. Oricum are legatura cu faptul ca nu am folosit destul object value. Si ma pot intoarce oricand la commit-ul cu pricina sa caut in ce a constat precis bug-ul.

Edited by wolfenste, 05 September 2018 - 16:13.

final LE: Mda, problema provenea de la un index gresit. Si intamplarea facu ca eu sa verific in test fix primul element care ar fi generat problema. Am presupus ca isMapAvailable asteapta index de Harta (doh) pe cand astepta o pozitie (egala cu index + 1). Am crezut ca voi avea nevoie de numere 1 - 9 si nu 0 - 8 la momentul definirii. Oricum, si LoD a cam incurcat aici, ca ma obliga sa scriu functii care combina functii. Asta din punct de vedere al codului. Insa cum phpunit prinse alta exceptie facuta pentru alt test inca nu imi explic. Cred ca trebuie sa invat mai mult cum lucreaza phpunit.

Edited by wolfenste, 05 September 2018 - 16:53.

wolfenste, on 05 septembrie 2018 - 15:47, said:

Nu vad nimic nastrusnic in asta. Este chiar ce te-am batut la cap sa faci.

wolfenste, on 05 septembrie 2018 - 15:47, said:

Faptul ca Game decide castigatorul nu inseamna ca in Map nu ai voie sa ai metode ajutatoare (care fac calculele cu indecsi, daca sunt necesare).

Poti avea metode precum Map::isWinningMove(MapCoordinate, Symbol) - sau orice alta abordare vrei tu sa ai - metode pe care Game le apeleaza.

Eu personal vad problema asa: Game raspunde la intrebari legate de castogator in termeni de Player si GameStatus sau ce ai tu acolo, si Map raspunde la intrebari legate de castigator in termeni de Symbol.

Cat despre test, tu spui in asertie ca te astepti sa ai o \Exception, iar \RangeException este un tip de \Exception.

As mai adauga o asertie @expectedExceptionMessage. Asta ti-ar arata ca testul e gresit.

wolfenste, on 05 septembrie 2018 - 15:47, said:

Value Object.

OriginalCopy, on 05 septembrie 2018 - 19:47, said:

Pai mai discutasem o data asta si hotarasem ca MapCoodinate se va ocupa de conversia in index. #118
Ma gandeam atunci ca asa oricine va avea acces la index (si parea flexibil) prin MapCoordinate dar acum e clar ca altcineva in afara de Map nu prea ar avea de ce sa stie de index; ba mai bine sa nu stie. Chiar ca e bine ca nu e proiect mare.

Edited by wolfenste, 06 September 2018 - 10:27.

wolfenste, on 06 septembrie 2018 - 10:26, said:

Map e cel mai in masura sa decida ce index are o coordonata. De ce? Map este o colectie de coordonate, iar coordonatele pot fi in conflict unele cu altele.

Si in general, daca ai un tip de date, si o colectie de acel tip de date, orice implica interpretarea datelor dintr-un obiect in contextul intregii colectii e cel mai bine facut de colectie, pentru ca ea stie cel mai bine ce si cum. In cazul nostru, Map stie cel mai bine ce fel de harta este ea insasi, ce valori sunt valide pentru index, etc.

Ce am zis atunci e deci mai rau ca ce ne-am dat seama acum. Notiunea de "index" e o notiune interna lui Map.

Edited by OriginalCopy, 06 September 2018 - 10:45.

Vreau sa schimb putin structura de baza a programului. Se pare ca strategia pentru AI are nevoie sa stie cu ce simbol lucreaza (spre deosebire de strategia pentru jucatorul uman care doar preia de pe net o pozitie iar obiectul player uman cere lui game sa puna simbolul sau acolo). Cand se uita pe harta sa-si faca strategia, trebuie sa stie care este simbolul sau.

Deci, vreau ca jucatorul (generic) sa poata fi construit cu si o strategie null (acum e musai doar NextMoveProvider). Daca e null, inseamna ca Game face un player de tip AI iar acest player creeaza intern o strategie de tip AI plasandu-i simbolul cu care lucreaza. Asta ar insemna sa schimb putin testele, doar testele pentru constructori.

E ok?

Altfel strategy AI poate sa faca strategia pentru ambele simboluri iar player sa ia de acolo doar ce corespunde simbolului sau, dar asta inseamna munca dubla pentru strategy AI si nici nu prea mai seamana cu ce trebuie, strategie proprie adica. Si nu e flexibil.

Sau strategy AI ramane de tipul NextMoveProviderAI inca din start, dar nu e creat complet iar playerAI ii da simbolul si cere completarea. Nu prea imi surade ideea asta cu creare incompleta. De ce il mai primeste ca parametru daca oricum trebuie sa il modifice?

Sau strategyAI nu cara date, e doar un grup de metode de care se foloseste playerAI dar asta inseamna sa recalculeze totul de fiecare data cand are nevoie de o valoare.

Edited by wolfenste, 23 September 2018 - 11:44.

Aici: https://github.com/w...oveProvider.php iti lipseste o metoda care sa faca ceea ce spune numele interfeteie ca face: iti da urmatoarea mutare.

Deci adauga o metoda getNextMove().

Asa unifici cele doua strategii - ai si manual - si nu trebuie sa mai faci distinctie. Simplifica codul si il face mai omogen.

Lui NextMoveProvider ii dai in constructor simbolul, dar in constructorul lui Player verifici daca intr-adevar simbolul strategiei de joc (nextmoveprovider) se potriveste cu propriul simbol.

Ok, o sa revizuiesc asta. Alta problema. AI nu doar se uita pe harta. AI vrea sa analizeze anticipat in functie de urmatoarea mutare a oponentului, si poate chiar si urmatoarea. AI are nevoie sa modifice   harta, temporar , apoi sa stearga toate mutarile respective si sa retina, la final, cea mai buna mutare.

Daca NextMoveProviderAI (ce preia ca parametru $map ReadOnlyMap) creaza intern un obiect dintr-o clasa ce mosteneste pe Map adaugand metoda de erase (pentru celulele completate temporar) si la creare preia din parametrul sau $map (ReadOnlyMap) mutarile existente?

Nu imi vine in minte momentan alta metoda mai putin complicata. Dar in felul asta $map (cel de baza) e asigurat chiar mai mult decat il asigura restrictia ReadOnlyMap si rezolva si necesitatea asta interna.

Edited by wolfenste, 24 September 2018 - 11:29.