PHP - prosty projekt zbudowany zgodnie z MVC

pytanie zadane 3 maja 2018 w PHP przez Gambr Dyskutant (7,530 p.)
edycja 3 maja 2018 przez Gambr

To moja pierwsza implementacja tego wzorca. Pomyślałem, że zanim zabiorę się za tworzenie jakichś sensownych aplikacji, poproszę o wasz feedback na forum. Z góry dzięki za wszystkie odpowiedzi i rady

Tutaj github

2 odpowiedzi

odpowiedź 4 maja 2018 przez Mariusz08 Maniak (62,300 p.)
wybrane 4 maja 2018 przez Gambr

Najlepsza

1. Dlaczego klasa Session jest abstrakcyjna? Nie używasz w niej żadnych metod abstrakcyjnych, a więc trochę to bez sensu.

2. Jak już kolega powiedział, wcięcia. Tak tylko powiem, że kod na githubie rozlatuje się dlatego, że w atomie inaczej wygląda "tabowanie" (tam ma rozmiar 4 spacji, na githubie więcej).

3. Jeśli chcesz pisać dobry kod, zainteresuj się PSR. Jest to standard, bardzo popularny w środowisku programistów PHP na całym świecie.

4. https://github.com/boring-leon/todo/blob/master/php/libs/DataBase.php#L14 <- Po co Ci tutaj małpa, jeśli robisz ładną obsługę try..catch?

5. PDO

6. Nie widzę bindowania, możliwy atak

7. https://github.com/boring-leon/todo/blob/master/php/libs/View.php#L10 <- brakuje deklaracji widoczności metody; dopisz public

8. https://github.com/boring-leon/todo/blob/master/php/libs/View.php#L33 <- możesz przenieść powyżej bloku try; spróbuj dopisać własny wyjątek

9. https://github.com/boring-leon/todo/blob/master/php/libs/View.php#L43 <- wcześniej używasz składni bez nawiasów; zdecyduj się na coś

10. https://github.com/boring-leon/todo/blob/master/php/Models/LoginModel.php#L30 <- else w tej samej linii co koniec ifa

11. https://github.com/boring-leon/todo/blob/master/php/Models/StartModel.php#L17 <- jeśli w pliku znajduje się sam kod PHP nie musisz używać ?>

komentarz 4 maja 2018 przez Assasz Nałogowiec (30,460 p.)

Nie jestem pewny, czy wcięcia są winą Atoma. Moje projekty pisane w nim niczym się nie różnią na GitHubie od projektów pisanych w PHPStorm.

Ogólnie to ten kod jest strasznie nie estetyczny i zniechęcił mnie do dalszego czytania. Trzeba popracować nad czytelnością i wykuć PSR-1 i 2 na blachę autorze pytania ;) W zasadzie PSR-4 też, bo widzę, że nie ma autoloadera.

komentarz 4 maja 2018 przez Mariusz08 Maniak (62,300 p.)

Atom sam robi wcięcia (jeśli piszemy coś w metodzie,klasie, interfejsie), czyli 4 spacje. Prawdopodobnie autor pytania miał te 4 spacje, potem je usunął, użył tabulatora i stąd te różnice - tab i 4 spacje w atomie wyglądają tak samo, a github inaczej je interpretuje.

komentarz 4 maja 2018 przez Gambr Dyskutant (7,530 p.)

Klasa Session jest abstrakcyjna bo tworzenie jej instancji nie ma sensu

komentarz 4 maja 2018 przez Mariusz08 Maniak (62,300 p.)

Ale nawet jeśli jakimś cudem ktoś stworzy instancję tej klasy (co raczej nie powinno się zdarzyć) to nie będzie mógł korzystać z żadnej z metod, gdyż wszystkie są zadeklarowane jako statyczne.

komentarz 4 maja 2018 przez Gambr Dyskutant (7,530 p.)

A co do konstruktorów, to każdy powinien mieć deklarację widoczności, tak?

1	komentarz 4 maja 2018 przez Mariusz08 Maniak (62,300 p.) Każda metoda powinna mieć zadeklarowaną widoczność

2	komentarz 4 maja 2018 przez Assasz Nałogowiec (30,460 p.) Jeśli koniecznie chcesz zabezpieczyć klasę przed tworzeniem instancji, możesz też ustawić konstruktor jako prywatny. Klasy abstrakcyjne z założenia służą do tego, aby inne klasy po nich dziedziczyły.

komentarz 4 maja 2018 przez Gambr Dyskutant (7,530 p.)
edycja 4 maja 2018 przez Gambr

A czy dobrym pomysłem jest zrobienie singletona z klasy Bootstrap? Tzn z tej głównej klasy, która obsługuje requesty i ładowanie kontrolerów

odpowiedź 3 maja 2018 przez Szymon Ciompała Mądrala (6,280 p.)

Tak na szybko:

1. zobacz sobie jakiś system szablonów np twig bardzo uprzyjemnia prace :)

2. Było by dobrze jakbyś lepiej formatował kod, bo czasami aż ciężko sie połapać co z czym się dzieje.

3. Jak na początek kodzik w miarę spoko :) Większych wad nie widzę. Troche mnie kuje w oczu ze plik bootstrap dziedziczy po kontrolerze ? Co chciałeś tym uzyskać ?

komentarz 3 maja 2018 przez Gambr Dyskutant (7,530 p.)
edycja 3 maja 2018 przez Gambr

Dzięki za opinię :) Co do Twoich rad to:

1. Twing był kolejnym celem do implementacji, ale najpierw chciałem sprawdzić czy w ogóle idę w dobrą stronę czy całkowicie źle rozumiem MVC

2. W atomie kod jest inaczej sformatowany i na githubie są jakieś dziwne wcięcia nie mam pojęcia dlaczego

3. Podejrzewam, że grzebałem przy kontrolerach a potem wróciłem do bootstrapa żeby coś zmienić i bezmyślnie napisałem extends Controller. Zaraz to poprawię

komentarz 3 maja 2018 przez Szymon Ciompała Mądrala (6,280 p.)

Pobierz sobie phpstorm. Za nic nie badziesz chciał wrócić do atoma jeżeli będziesz pisał większe aplikacje w php. Do tego widze chyba ze nie używasz namespace ? Poczytaj o tym i dodaj do swojego projektu autoloadera z composera. Nie bedziesz musiał recznie importowac plików z klasami :). https://getcomposer.org/doc/01-basic-usage.md . I do tego jak dodasz twig to i dodaj jakiś routing np fast route :)

komentarz 3 maja 2018 przez Gambr Dyskutant (7,530 p.)

function loadAllFrom($path){

            $fileList = glob($path.'/*.php');

            try{
                if(empty($fileList))
                    throw new Exception("No php files under ".$path);

                foreach ($fileList as $file)
                    require_once $file;
            }
            catch(Exception $e){
                exit($e->getMessage());
            }
        }

Czy taka funkcja jest ok jeżeli chodzi o ładowanie wszystkich plików php z danego folderu?