Współpraca Entity Opcje

[athabus]

Mam takie pytanie teoretyczne, które od czasu do czasu pojawia się w moich projektach. Najlepiej chyba będzie opisać na przykładzie.

Piszę aplikację sportową (konkretnie dla pływaków) we frameworku Symfony i w niej mam między innymi dwa obiekty Entity:
- User
- Interval

Użytkownik ma pewne cechy definiowalne - np. prędkość bazową (dajmy na to 2:00/100m), modyfikatory prędkości (np. gdy ma płetwy to płynie 10s/100m szybciej niż bez etc).

Interwał to cegiełka z której buduję treningi do wykonania. Czyli każdy interwał posiada między innymi:
- dystans do pokonania
- prędkość wyrażoną jako prdkość bazowa +/- x sekund.
- listę modyfikatorów do zastosowania - czyli np. dany interwał użytkownik ma płynąć w płetwach

I teraz dochodzimy do tego co mnie interesuje. Mam konkretny interwał w którym jest płyń z prędkością bazową - 5s oraz użyj płetw. Mam konkretnego użytkownika, dla którego prędkość bazowa to 2:00, a modyfikator płetw daje - 10s. Czyli finalnie ten konkretny użytkownik ma popłynąć ten konkretny interwał w tempie 2:00 (prędkość bazowa użytkownika) - 5s (ustawienie interwału) - 10s (bonus za płetwy) - razem daje 1:45/100m.

Pytanie brzmi - jaki obiekt powinien obliczyć tą konkretną wartość, żeby było prawidłowo i zgodnie ze sztuką.

Interwał raczej nie powinien wiedzieć o użytkowniku i vice versa. Robienie tych samych obliczeń w 50 miejscach (np. w kontrolerach) jest bez sensu i trudne w utrzymaniu. Osobiście w tym celu stosuję usługę, którą sobie nazwałem WorkoutService i mam tam takie funkcje jak np getIntevalPace.

Zastanawiam się czy to jest dobre rozwiązanie, czy może jakoś inaczej powinienem to ogarniać.

[Crozin]

1. Przede wszystkim problemem tej klasy jest to, że musiałeś użyć aż trójelementowej listy by opisać co ona w ogóle robi. A robi zdecydowanie za dużo i powinna zostać rozbita na wyspecjalizowane obiekty.
2. Taka trochę ogólna uwaga co do tworzenia całych grafów encji, np. w createNewWorkoutBlock.

[PHP] pobierz, plaintext 
$block=new WorkoutBlock();
$task=new WorkoutTask();
 
$interval=new WorkoutInterval();
 
$rest=new WorkoutInterval();
$rest->setType(WorkoutInterval::TYPE_REST);
$rest->setDuration(10);
 
$task->addInterval($interval); // wylicza m. in. pozycję oraz ustawia dwu/jednostronną referencję
$task->addInterval($rest); // j/w
$block->addTask($task); // j/w
$workout->addBlock($block); // j/w
 
// raczej nie powinno znajdować się to już w tej metodzie, ale jak już to wystarczy
$this->em->persist($block);
$this->em->flush();
 
return $block;
[PHP] pobierz, plaintext 

Tyle wystarczy, logika wyliczania pozycji i innych tego typu rzeczy może spokojnie zostać zamknięta w encji nadrzędnej.
3. Trochę analogicznie jest z klonowaniem. Też przeniósłbym to do metod w ramach encji. Swoją drogą uważaj przy korzystaniu z clone w przypadku encji. Można z tego korzystać, ale niesie to za sobą kilka implikacji (patrz: dokumentacja). Zdecydowanie polecałbym Ci zrobić całość klonowania ręcznie. Zdecydowanie upraszcza i bardziej klarowne staje się wtedy to czy robimy płytką czy głęboką kopię.
4. Encje traktuj tylko jako coś co wykorzystywane jest niemal wyłącznie w komunikacji z bazą danych, nie jako fundament obliczeń dla całej apki (niestety takie podejście jest bardzo popularne w sieci/dokumentacji symfony).
5. Uwzględniając powyższy punkt oraz to co widać w kodzie, wydaje się, że tutaj encje są fajnie zaprojektowane, ale dalsze wyliczenia dla faktycznej logiki aplikacji nie powinny już być wykonywane na nich, ale na wyspecjalizowanych obiektach (nowych klas) z części nazwijmy to domenowej aplikacji. Jaka będzie różnica pomiędzy tymi dwoma zestawami obiektów? Przykładowo te z części domenowej w ogóle nie potrzebują żadnej wiedzy na temat jakiegoś użytkownika, który widzę, że się tam przewija. Nie potrzebują też żadnych właściwości z serii itemOrder - potrzebują, by kolekcje podrzędnych obiektów były po prostu uporządkowane (co PHP-owe tablice zapewniają).
5. getWorkoutDistance - te trzy zagnieżdżone foreache powinny być raczej zastąpione jednym, gdzie źródłem danych będzie jakiś iterator. Co lepsze taki iterator mógłby od razu w momencie swojego tworzenia mieć przekazane jakiego typu interwałów w ogóle (nie)chcemy z niego uzyskać (3 zagnieżdżone foreache z ifem zmieniają się w jednego foreacha). A co jest jeszcze lepsze? Ta metoda w ogóle może być spokojnie przeniesiona do nowej klasy WorkoutZCzęściDomenowej.
6. W angielskim nie ma słówka "brutto". :-P
7. Analogicznie jak z getWorkoutDistance wydaje się, że getEstimatedWorkoutDuration, getWorkoutDuration, getBlockDuration czy getTaskDuration można przenieść do wybranych klas z części domenowej.
8. Te wszystkie duration nie wyrażałbym w intach czy floatach, a w DateInterval. Zdecydowanie lepiej będzie nadawać się do reprezentowania takiej wartości - chociażby ze względu na obsługę jednostek, co wydaje się bezwzględnie konieczne tutaj.
9. Te wszystkie metody z vcsettingsami też wydają się do przeniesienia.
10. Unikaj rzucania wyjątków klasy Exception - użyj bardziej wyspecjalizowanej, a jeżeli takie nie ma (która pasowałaby do danej sytuacji) stwórz swoją.
11. Mam wrażenie, że do wyliczania wielu z tych interwałów/modyfikatorów/temp i innych świetnie mogłoby nadać się podejście bazujące na wzorcu wizytatora (ang. visitor pattern). Tylko z tym wstrzymałbym się na początku, bo kto wie czy nie jest to już właśnie to zbytnie rozdmuchanie o którym wcześniej pisaliśmy.

I tak chyba doszedłem do końca tej klasy, co do której wydaje się, że... może ona spokojnie być kompletnie zaorana! (IMG:style_emoticons/default/tongue.gif) Wtedy też problem z pierwszego punktu sam się rozwiąże.

Ten post edytował Crozin 8.06.2018, 06:16:56