CMS - Ocena projektu, Laravel

0

Cześć, od jakiegoś czasu tworzyłem swój większy projekt dzięki któremu miałem poznać lepiej framework'a PHP jakim jest Laravel 5.4.

Myślę że większość funkcjonalności aplikacji zostały skończone i działają. Jednak teraz chciałbym się dowiedzieć co o mojej aplikacji sądzą inni, chętnie poznam waszą opinię (Szczególnie tych którzy także programują w Laravel).

Aplikację można znaleźć na moim profilu Github.
https://github.com/jzebala/myApp

myApp - CMS (niestety wymagana jest instalacja)

Wszystkie kroki instalacji są podane w pliku README.md jednak gdyby coś nie działało pytać śmiało.

Z góry dziękuje za każdą odpowiedź.

1
  • Usuń z repozytorium composer.lock - on ma reprezentować stan na danej maszynie, a nie stan w repozytorium. (nieaktualne; patrz: komentarze pod tym postem)

  • Nie commituj zakomentowanego kodu (np. w main.css).

  • Wejdź sobie do katalogu /public - teraz, najlepiej na drugim monitorze, otwórz osobne okienko z katalogiem /public/myappblog. Widzisz podobieństwo?

  • /public/tinymce powinieneś wydzielić do jakiegoś odrębnego katalogu w stylu /public/3rdparty albo cokolwiek, aby nie zaśmiecało głównej przestrzeni.

  • Niepotrzebnie dużo zacommitowałeś boilerplateowego kodu w stylu całego katalogu /tests, gdzie - z tego co widzę - nie ma ani jednej Twojej linijki kodu. Albo spora część plików w /config - rozumiesz chociaż ich zastosowanie czy raczej są, więc ich nie ruszam?

  • Zdecyduj się na jednorodne nazewnictwo. Z jednej strony /app/Post.php, a z drugiej /app/LoginLogs.php (liczba pojedyncza-mnoga).

  • Modele powinieneś trzymać w odrębnym katalogu, np. /app/models.

  • /app/Comments.php - wydaje Ci się, że komentarz Time added post do funkcji o nazwie commentTime wnosi cokolwiek? Moim zdaniem nie wnosi niczego, czego nie można by się domyślić w ułamek sekundy patrząc na samą nazwę metody, zatem jest zbędny. Pomijam już fakt, że time added post to nawet nie jest kali jeść, kali chcieć, tylko jakieś zdanie, o którego wypowiedzenie można by posądzać najwyżej dziewięćdziesięcioletnego hindusa. Returns time (in seconds) since the post has been created. i od razu ładniej ;-) Tym niemniej, akurat tutaj: zbędnie.

  • Pousuwaj te wszystkie The attributes that are mass assignable. i inne automatycznie generowane teksty. Jeśli ktoś się zna na Laravelu, doskonale wie, za co odpowiada pole fillable, a jeśli ktoś się nie zna, i tak mu to zdanie nic nie powie.

  • Co prawda w Laravelu jest to temat nie do końca ustalony, lecz polecę i tak: twórz repozytoria (repository pattern, nie pomyl tego z GitHubem :P). Wygodniej testuje się kod, jest bardziej przejrzysty.

  • /app/Providers/AppServiceProvider.php - Schema::defaultStringLength(191); // *add wtf?

  • /app/Calendar.php - to w końcu jest to kalendarz czy wydarzenie? Nazwa pliku sugeruje jedno, jego zawartość - drugie.

  • Zapoznaj się z pojęciem model presenter.

  • /app/Http/Controllers/BlogController.php - kolejne przykłady zbędnych komentarzy. Nazwa metody index jest tak niejasna, że bez komentarza Display a listing of the resource. nie wiadomo, co ona robi? ;-)

  • $config = BlogConfig::firstOrFail(); // get first config record. - tak jak wyżej: firstOrFail raczej nie powoduje wystrzelenia nowego Apollo w kosmos, tylko zwraca ostatni rekord, co mówi wprost nazwa tej funkcji, więc komentarz jest całkowicie niepotrzebny. Od tego momentu już pomijam jakiekolwiek wzmianki o komentarzach: załóż, że u Ciebie praktycznie wszystkie są zbędne. Uwaga: PHPDoce zostawiaj.

        if($post->publish != 0){
            return view('templates.myappblog.show', compact('post', 'config'));
        }else{
            return redirect('/');
        }

Zwyczajowo ify z klauzulą else konstruuje się tak, aby nie powstawała podwójna negacja. Czyli nie if (A != B) ..., tylko if (A == B). Ale tylko wtedy, jeśli jest else.

  • Pisz w IDE, które potrafi automatycznie formatować kod - albo naucz się pisać go w jednorodnej stylistyce (np. metoda updateAvatar).

  • Tworzenie generycznego kontrolera jest imo ryzykowne. Co w sytuacji, gdy - z jakiegokolwiek powodu - zechcesz mieć kontroler, który nie będzie mógł/potrzebował/whatever mieć traita DispatchesJobs? Teoretycznie wystarczy, aby w takiej sytuacji nie dziedziczył z Twojego Controller (który powinien być klasą abstrakcyjną, tak btw.), lecz z drugiej strony wtedy zastanawiające jest dla czytelnika dlaczego ten kontroler nie dziedziczy z Controller???.

dalej już nie zerkam, imho póki co wystarczy

Podsumowanie

Tragedii nie ma, lecz spójrzmy na to obiektywnie: Twoja aplikacja to jest typowy CRUD, nie ma tutaj żadnych fajerwerków, którymi można by się popisać. Dodaj do niej jakieś funkcjonalności, których nie będzie mieć kartka papieru i długopis, to dopiero będziesz mógł powiedzieć, że uczysz się zastosowania frameworka :)

Tym niemniej: keep it up, jesteś na dobrej drodze.

0

@Patryk27 composer.lock się nie usuwa, to on daje pewność 100% że będą na serwerze te same wersje bibliotek co lokalnie testowane. Napisałes, że composer.json się wpisuje taaaa w 99% przypadkach to dajesz gwiazdki albo przybliżenia, żeby lokalnie po użyciu "composer update" zaktualizował do nowszych wersji ale nie koniecznie milestones.

0
Patryk27 napisał(a):
  • Co prawda w Laravelu jest to temat nie do końca ustalony, lecz polecę i tak: twórz repozytoria (repository pattern, nie pomyl tego z GitHubem :P). Wygodniej testuje się kod, jest bardziej przejrzysty.

W laravelu się tego już nie stosuje :D A przynajmniej parę miesięcy temu twórca frameworka ogłosił, że jest to złe i lepiej robić od razu małe metody w modelach zamiast tworzyć interfejsy i budować dodatkową złożoność kodu, ani do testowania nic to nie wnosi lepszego a wydajność tylko na tym traci :)

0

Aktualnie sam piszę projekt w Laravelu i właśnie miałem spory problem z ustalaniem czy wykorzystywać repozytoria czy nie. Laravelowa społeczność, z tego, co wyczytywałem, była podzielona, więc postanowiłem się z tym pobawić i imho póki co wychodzi na plus. Tak, tworzy to dodatkową warstwę (pozornie zbędną), lecz moim zdaniem umożliwia całkiem ładne odseparowanie modeli od tego, co te modele zwraca :)

Może powinienem dodać fakt, że nie jestem fanem wzorca active record, co pewnie nieco zmienia podejście do tematu :D

1 użytkowników online, w tym zalogowanych: 0, gości: 1