[rtrv]

Mirki, uczę się #php i zrobiłem sobie zadanko zapostowane jakiś czas temu na mirko, starając się stosować w praktyce rzeczy, których ostatnio się nauczyłem. Mógłbym prosić kogoś mądrego o rzuceniem okiem na kod (nie ma tego dużo) i wytknięcie błędów / złych praktyk?

[[Link do githuba]](https://github.com/retrowaver/Transiter)

Uwagi: Starałem się stosować MVC (ale jakby bez View, bo tu sprowadza się tylko do wyświetlenia JSONa). Pierwszy raz ever pisałem testy i nie wiem czy w obecnej postaci mają sens (na pewno brakuje paru, zwłaszcza sprawdzających bazę danych).

#naukaprogramowania #codereview #programowanie

[KociPazur]

@rtrv: na pierwszy rzut oka:
użyj php7, typowania zmiennych, za dużo ifów w controllerze, zrób z tego jakąś fabrykę/strategię.
Za dużo wzorca IFLETON!
Poczytaj sobię zasady SOLID, KISS, DRY

[progreso]

@rtrv: i to? defined("DIRECT_ACCESS") OR exit;

[h0lend9r]

@rtrv: autoload zalatwiaj comoposerem (psr-4)

[qwelukasz]

@rtrv: formatowanie (psr) leży i kwiczy

[KociPazur]

@rtrv:
header('Content-Type: application/json');
echo json_encode($json);

wynoś takie rzeczy to metod osobnych, najlepiej jakąś własną klasę Response utwórz

[KociPazur]

if ( ! @isset($result->rows[0]->elements[0]->distance->value))

CO TAM ROBI STFU OPERATOR!

[KociPazur]

$apiUrl = 'https://maps.googleapis.com/maps/api/distancematrix/json?units=metric&origins=%s&destinations=%s&key=%s';

do słownika jakiegoś, jako const

[qwelukasz]

@rtrv:

https://github.com/retrowaver/Transiter/blob/master/src/Controller/ReportsController.php#L83

for ($day = (int)$firstDayOfTheMonth->format('j'); $day <= (int)$now->format('j'); $day++)

[qwelukasz]

@rtrv:

https://github.com/retrowaver/Transiter/blob/master/src/Model/GoogleApiModel.php

To nie powinien być model, bardziej jakiś

[rtrv]

@KociPazur: @progreso: @h0lend9r: @qwelukasz: Dzięki wielkie!

[Benzen]

Starałem się stosować MVC (ale jakby bez View, bo tu sprowadza się tylko do wyświetlenia JSONa)

@rtrv: I jakby bez Model ( ͡° ͜ʖ ͡°) Całą logikę z kontrolerów wynieś do osobnych serwisów.
I formatowanie kodu ( ͡° ʖ̯ ͡°)

[MQs]

@rtrv:
1. Jako root domeny z plikiem index.php zrób katalog public projektu. Nie będziesz potrzebował tych sztuczek z defined("DIRECT_ACCESS") OR exit;. Skrypt niech sobie zrobi require_once dirname(__DIR__) . '/src/bootstrap.php'; czy co tam mu trzeba.
2. Kontrolery nie są od przetwarzania danych - to jest część aplikacji, którą byś bez problemu wymienił w przypadku innego sposobu ich dostarczania (komenda+parametry z konsoli, import z pliku, pobrane wcześniej z innego serwisu...itp). W

[Jare_K]

defined("DIRECT_ACCESS") OR exit;

@rtrv: Jak pisał kolega wyżej, wyrzuć kod poza public. A nawet jeśli ktoś odpali skrypt klasy bezpośrednio z poziomu www, to co z tego? Przecież nic się nie wykona, nie zainicjuje. Skrypt się zakończy. Nic nie wycieknie. Więc niepotrzebnie to wrzucasz i powodujesz, że kod nie będzie re-używalny w innym projekcie, który takiej stałej nie definiuje.
Do tego zasada: skinny controllers, fat models. Tylko model to nie

[rtrv]

@Benzen: @MQs: @Jare_K: Dzięki, super :) Przebijam się przez to wszystko powolutku

[rtrv]

Nie gniewajcie się, że wołam nagle po czterech tygodniach, ale poprawiłem (mam nadzieję) apkę wg Waszych uwag. Moglibyście jeszcze raz rzucić okiem?

Link / link do poprzedniej wersji

@KociPazur: @progreso: @h0lend9r: @qwelukasz: @Benzen: @MQs: @Jare_K:

[Jare_K]

@rtrv: Jest postęp. Dobry kierunek! ( ͡° ͜ʖ ͡°)
Nie mam czasu przeglądać wszystkiego, ale tak po łebkach to bym sugerował wyrzucić zawartość tych złożonych funkcji kontrolerów do serwisów. Kontroler im mniejszy tym lepszy.
Najlepiej, żeby wywoływał metodę jakiegoś serwisu. Wtedy jak będziesz chciał przenieść swój projekt na jakikolwiek framework, to zrobisz to w mig kopiując wywołania metod serwisu do akcji kontrolera, zamiast całe ciała funkcji,

[rtrv]

@Jare_K: Dzięki wielkie!