prośba o ocenę kodu

Question

Witam proszę o ocenę kodu (projekt jest w SF4 i Vue) ewentualnę wskazówki link . Krytyka mile widziana:) co tu można zmienić aby bardziej trzymać się SOLID i  żeby projekt był mniej uzależniony od frameworka?(żeby można było go przenieść łatwo np. do laravela). Rozbiłbym bym te serwisy jeszcze na mniejsze klasy tylko nie mam pomysłu:)

dodam że nie dopiero się uczę, proszę o wyrozumiałość.

Answer 1

1. Nie przestrzegasz psr. Polecam zainstalować phpcs, bo nigdzie nie ma odpowiedniej ilości przejść do nowej linii.
2. Używaj declare strict types.
3. Powinieneś pomyśleć o lepszej nomenklaturze. CurrencyProvider ok, ale co to jest getData, to już wiesz tylko Ty.
4. Nie commituj komentarzy. Jeśli chcesz mieć wgląd na starsze rzeczy, to zawsze masz historię gita.
5. W Converterze brakuje Ci typowania. To dość istotna kwestia jeśli rozchodzi się o hajs.
6. Nie commituj do repo wrażliwych danych takich jak access_key. Powinieneś je trzymać w pliku z parametrami. 
7. Nie korzystasz z formularzy. Powinieneś ​​​​​​
8. Nie musisz korzystać z json_decode. Dane są dostępne w polu request. 
9. Masz całą logikę w kontrolerze. Nie powinno tak być. Przenieś ją do jakiegoś serwisu. 
10. Pisz dokumentację nad metodami. 
11. Szkoda że nie ma żadnych testów. Twoje metody są na tyle małe że spokojnie można byłoby pokryć je testami jednostkowymi. 

Powodzenia dalej 

Comments

co dokładnie masz na myśli w pkt9 ?

---

Controller to kiepskie miejsce na wykonywanie logiki. Powinna być wykonywana w odpowiednio nazwanej klasie. Łatwiej to testować, łatwiej się odnaleźć w kodzie.

---

@Ehlert,

Dziękuję za poświęcony czas!

- zmieniłem CurrencyProvider na CurrencyDataProvider oraz getDate na getCurrencyCodes;

- usunąłem zakomentowany kod:);

- faktycznie typowane jest kluczowe dopiero do dostrzegłem kiedy to zauważyłeś:) kluczowy typ dla moich danych to float, jak ustawiłem integer to cała apka się posypała:) Dzięki za tą podpowiedź!;

- przeniosłem access_key do env, pobrałem dependency donenv,aby korzystać z zmiennych z env,;

- pobrałem squizlabs/php_codesniffer  skonfigurowałem z phpstormem, nie było aż tak dużo tych przejść do nowej linii;P w CS mam lecieć z standardem PSR-2 ?;

- co do używania json_decode to nie bardzo rozumiem jeśli go usunę, jak później korzystać w moim kodzie odnośnie danych z requestu:);

- dodałem parę opisów nad metodami ( dość krótkich :) );

- przeniosłem logikę do osobnego serwisu z przeładowanego kontrolera, oraz tworzenie nowego obiektu encji wydzieliłem do metody new/save w repository;

- nie korzystam z formularzy bo te dane odbieram poprzez axiosa(biblioteka do requestów) po stronie frontedu napisanego w vue.js (jest w assetach/js), czy chodzi o coś innego z tymi formularzami?

Mam jeszcze takie pytanie czy podając wartość $money w:;

$converterService->setAmount("$money");

jak jest najlepiej: $money, "$money", "{$money}" ?

Mógłbyś rzucić jeszcze okiem czy jest progres? projekt -> link

---

Korzystaj ze standardu Psr2. Poczytaj jakie są jeszcze. W internecie są gotowe konfiguracje PHPCSa.

<?php
$data = json_decode($request->getContent(), true); //nie
$data = $request->request; //tak

Pole request to nic innego jak $_POST.

Repozytorium nie powinno odpowiadać za tworzenie. wyciąganie, zapis. Update jest kwestią dyskusyjną. Tworzenie na pewno nie.

To że na froncie masz formularze we Vue i korzystasz z axiosa to nie znaczy, że nie powinieneś mieć ich na backendzie. W Symfony formularze (Types) to nic innego jak swego rodzaju filtr, sposób opisu danych. Pomaga w walidacji.

Co do tego settera to nie wiem skąd tam Ci się wzięło "". Jeśli spodziewasz się inta, to przekaż do funkcji zmienną int. Bez "".

Dodaj pustą linię:

• <?php
• Po namespace
• Po ostatnim use 
• W docblocku po ostatnim @param
• Po każdej klamrze } kończącej metodę.
• Na końcu pliki

Bez tego ledwo się czyta 

---

@Ehlert ok, a tak to dobrze teraz to wygląda po poprawkach? kontroler nie jest teraz przeładowany?

to gdzie mam umieścić ten zapis encji? to nie ma większej logiki tylko sam zapis danych?

tylko czemu phpStorm mi podpowiada nazwę zmiennej obok jeśli używam "$value"