Witaj Gościu! ( Zaloguj | Rejestruj )

Forum PHP.pl

 
Reply to this topicStart new topic
> [PHP] Logowanie, zwrot/wypozyczenie ksiazek OOP - mocno początkujący
mrpickles
post 14.10.2019, 18:03:39
Post #1





Grupa: Zarejestrowani
Postów: 8
Pomógł: 0
Dołączył: 14.10.2019
Skąd: Białystok

Ostrzeżenie: (0%)
-----


Cześć,
Zaczynam naukę OOP i w napisałem skrypt który umożliwia rejestracje/logowanie/zwrot/wypożyczenie książek.
Mam świadomość istnienia SOLID oraz PSR - czytałem o PSR 1 / 2 i staram się przestrzegać.
Nie znam MVC ani testów np. PHPUnit - to będą kolejne kroki w nauce.

Czy taki skrypt można określić obiektowym, czy jest to jakaś hybryda? Zanim pójdę dalej jw chciałbym nauczyć się dobrych nawyków i w prawidłowy sposób posługiwać się OOP.

Skrypt nie wszedł na forum, umieściłem na githubie

Link do GitHub

Moje dodatkowe pytania:

1. Tworzenie obiektu Database w konstruktorze innych klas, czy jest to prawidłowe?
2. Metoda checkCredentials klasy log - czy nie narusza zasady pojedynczej odpowiedzialności? Czy nie powinna np. wyszukiwać użytkownika,a następnie inna metoda powinna weryfikować dane?

Dzięki za wszystkie uwagi i poświęcony czas smile.gif
Go to the top of the page
+Quote Post
viking
post 14.10.2019, 18:38:18
Post #2





Grupa: Zarejestrowani
Postów: 5 397
Pomógł: 916
Dołączył: 30.08.2006

Ostrzeżenie: (0%)
-----


Najpierw zastosuj autoloader, potem wyrzuć wszystkie htmle z plików z klasami i będzie można zacząć czytać. Jaki jest sens database?


--------------------
Go to the top of the page
+Quote Post
ohm
post 14.10.2019, 20:13:08
Post #3





Grupa: Zarejestrowani
Postów: 515
Pomógł: 124
Dołączył: 22.12.2010

Ostrzeżenie: (0%)
-----


klasa Log jest dość mocno myląca, brak namespace'ów (autoloadera j/w)
metoda "insert" z form tez jest, ze tak powiem, dosc dziwna, ponieważ insert w formularzu a insert do bazy danych to dwie rozne odpowiedzialnosci.

  1. foreach ($b as $a){

Coś takiego jest w ogóle niedopuszczalne (wg mnie)

Ogólnie wydaje mi się że najlepiej byłoby jak byś zaczął wdrażać się w jakiś framework (np. symfony) + narzędzia typu cs fixer, i na tej podstawie zacząłbyś wyrabiałbyś sobie dobre nawyki.
Go to the top of the page
+Quote Post
mrpickles
post 15.10.2019, 20:06:49
Post #4





Grupa: Zarejestrowani
Postów: 8
Pomógł: 0
Dołączył: 14.10.2019
Skąd: Białystok

Ostrzeżenie: (0%)
-----


1. Dodałem autoloadera
2. Nie znam smarty ani innego template engine, więc HTML wrzuciłem w echo, jeśli to miałeś na myśli. Czy chodziło o rozdzielenie o tyle o ile to możliwe np. index i index_view, ale bez żadnego template engine?
3. Klasa Database w zamyśle miała być universalna do crud, nie wszystko jest z niej wykorzystywane w tym projekcie. Czy korzystając z PDO powinna ona tylko nawiązywać połączenie, a wszelkie operacje na bazie powinny być już napisane w innych klasach?
np. funkcje registerUser w klasie RegistrationForm wygląda następująco:
  1. public function registerUser($data){
  2. if($this->isError==false){
  3. $this->connection->query("INSERT INTO uzytkownicy VALUES(NULL,:nick,:email,:password)");
  4. $this->connection->bind(":nick",$data['nick']);
  5. $this->connection->bind(":email",$data['email']);
  6. $this->connection->bind(":password", password_hash($data['password'],PASSWORD_DEFAULT));
  7. $this->connection->execute();
  8. }

i rzeczywiście dostrzegam trochę niepotrzebność kodu - korzystając z metod Detabase jedyne co zaoszczędziłem w tym przypadku to przy bindowaniu nie daje parametru INT, STR etc, a w pozostałych linijkach mogłem korzystać bezpośrednio z PDO. Jednak była to klasa tworzona w ramach tutorialu OOP, z którego wywnioskowałem właśnie taki sposób działania.

3. Zmieniłem klase log na LoginForm
4. Dodałem namespace, przez co przy autoloaderze się musiałem namęczyć smile.gif
5. Zmieniłem insert na RegisterUser, jeśli miałeś na myśli mylącą nazwę funkcje - funkcja to zakłada konto, wsadzając rekordy do bazy.
5.
  1. foreach ($b as $a){

zmieniłem na inne zmienne, kiedyś ucyzłem się pythona i na potrzeby tymczasowej iteracji jak dobrze pamiętam dopuszczano właśnie takie oznaczanie zmiennych.
6. Dodałem komentarze do metod w RegistrationForm

Tak, będę ogarniał frameworka, postawiłem na Symfony, ale najpierw chciałem dla świętego spokoju uporządkować to co zrobiłem i chciałem zapoznać się z jakimś template engine, żeby było przejrzyściej, ale może to niewłaściwa kolejność.

Nowe pliki pod tym samym linkiem.

Dzięki za wypowiedzi.

Go to the top of the page
+Quote Post
athabus
post 15.10.2019, 21:03:22
Post #5





Grupa: Zarejestrowani
Postów: 893
Pomógł: 45
Dołączył: 2.11.2005
Skąd: Poznań

Ostrzeżenie: (0%)
-----


Niestety trochę błądzisz, ale moim zdaniem to bardzo dobra decyzja, że próbujesz coś napisać samodzielnie zamiast uczyć się frameworka. Frameworki niestety uczą złych nawyków jeśli nie umiesz w porządne OOP.

To od czego bym zaczął na Twoim miejscu to composer. Composer dzisiaj to jest standard w każdym projekcie i warto go użyć już teraz choćby po to aby mieć autoloader.

Druga rzecz to PSR - pisałeś, że czytałeś i starasz się stosować, ale w kodzie tego zupełnie nie widać... Polecam zainstalować php-cs-fixer i odpalić w projekcie, a potem zobaczyć sobie jakimś diffem co zostało zmienione.

Mamy PHP 7.3, a w twoim klasach tego nie widać - nie ma typowania parametrów funkcji, typów zwracanych itp.

Z takich pierdół unikaj jak ognia stosowania if... else, a jeszcze bardziej zagnieżdżania ifów bo to zło wcielone.
Takich rzeczy jak to nie da się czytać
Kod
public function checkCredentials($data){
        $this->connection->query("SELECT * FROM uzytkownicy WHERE nick=:nick");
        $this->connection->bind(":nick",$data['nick']);
        $this->connection->execute();
        if($this->connection->rowCount()>0){
            if(password_verify($data["password"],$this->connection->singleResult()->haslo)){
                $this->isLoged=true;
                return $this->isLoged;
            }else{
            
            $this->error="Incorect nick or password";
            }
            
        }else{
            $this->IsError=true;
            $this->error="Incorect nick or password";
        }
    }


Zobacz o ile czytelniejsza byłaby ta funkcja po refactorze (użyłem wyjątków, ale możesz tu wstawić swoją logikę ze zwracaniem false itd):
Kod
public function checkCredentials(array $data): bool
    {
        $this->connection->query("SELECT * FROM uzytkownicy WHERE nick=:nick");
        $this->connection->bind(":nick",$data['nick']);
        $this->connection->execute();

        if($this->connection->rowCount() == 0) {
           throw new \Exception('User not found');
        }

        if(!password_verify($data["password"], $this->connection->singleResult()->haslo)) {
            throw new \Exception('Incorect user or password');
        }

        $this->isLoged = true;
        return true;
    }


Wewnątrz konstruktora (i ogólnie klas) nie powinieneś używać new bo to jest ukrywanie zależności. W dobrym kodzie wszystkie zależności powinny być wstrzykiwane w konstruktorze. Ale to może być dla Ciebie trudne gdy bo nie ogarniasz jeszcze zapewne wzorców projektowych typu fabryki, czy Dependency Injection. Niemniej postaraj się gdzie dasz radę raczej tworzyć klasy bez używanie w ich kodzie new. Czyli przykład:
Kod
/**
     * Źle
     */
    public function __construct()
    {
        $this->connection = new Database();
    }

    /**
     * Dobrze
     */
    public function __construct(Database $connection)
    {
        $this->connection = $connection;
    }


W php do komentarzy funkcji stosujemy php doc block, a jeszcze lepiej typowanie inputu/output + doc block jako uzupełnienie (np. o rzucane wyjątki, typu elementów w tablicy itp).

Jeśli myślisz na poważnie o kodowaniu, to zacznij w 100% stosować angielski - czemu funkcje są po angielsku, a baza po polsku? Pokracznie to wygląda.



To chyba na początek tyle.

Jak to ogarniesz to na Twoim miejscu spróbowałbym przerobić jakiś tutorial MVC żeby zrozumieć jak wygląda architektura prostej aplikacji w PHP. Robiesz bardzo wiele błędów i masz ogólnie źle zorganizowany kod. Przerobienie dobrego tutoriala na pewno pomoże.

Ogólnie szału nie ma, ale każdy kiedyś zaczynał, a widać że czytasz i starasz się rozwijać, więc to dobrze wróży. Już samo używanie gita i githuba na Twoim poziomie to bardzo dobry znak.Trzymam kciuki.

Ten post edytował athabus 15.10.2019, 21:04:54
Go to the top of the page
+Quote Post
mrpickles
post 17.10.2019, 21:00:47
Post #6





Grupa: Zarejestrowani
Postów: 8
Pomógł: 0
Dołączył: 14.10.2019
Skąd: Białystok

Ostrzeżenie: (0%)
-----


Tak wiem, błądzę muszę poukładać wiedze.

1. zrobiłem composera i dodałem autoloada z composera - rzeczywiście prościej niż ręcznie, podszkalam się z niego.
2. zrobiłem też php-cs-fixer - muszę tylko rozgryźć diffa bo chwilowo robię podgląd zmian ręcznie na klonie.
3. dałem type hinting
4. chyba zrozumiałem intencje z ifami i rzeczywiście wygląda lepiej, poprawiłem kod w tym zakresie jeszcze w pozostałych miejscach, nie wpadłbym na takie podejście, dzięki.
5. dependency injection wiem o co chodzi, dajemy jako argument konstruktora obiekt klasy, nie pomyślałem, żeby wykorzystać to do zrobienia połączenia z bazą danych, ale co do swojego połączenie w konstruktorze nie byłem przekonany, stad pytałem o to w pierwszym poście. ( nie tylko konstruktora oczywiście, ale generalnie jest konieczne jest dziedziczenie klas i jako argument metody w klasie która dziedziczy damy obiekt rodzica klasy )
6. co do polskiego w bazie wiem, ale obejrzałem się za późno, a nie o bazę w tym projekcie chodziło stąd rozstawiłem
7. zmieniłem komentarze na odpowiednie znaczniki, jeśli chodzi o docblock pisząc output i input masz na myśli meta tagi param i return w dokumentacji?
I w takim razie czy to byłby prawidłowy zapis?
  1. /**
  2.  * checking credentials
  3.  * @param array $argument1 array structure to verify credentials
  4.  * @return bool Return bool if credentials are correct
  5.  */
  6. public function checkCredentials(array $data) :bool
  7. {
  8. $this->connection->query("SELECT * FROM uzytkownicy WHERE nick=:nick");
  9. $this->connection->bind(":nick", $data['nick']);
  10. $this->connection->execute();
  11. if($this->connection->rowCount()==0){
  12. $this->isError=true;
  13. $this->error="Incorect nick or password";
  14. return false;
  15. }
  16. if(!password_verify($data["password"], $this->connection->singleResult()->haslo)){
  17. $this->isError=true;
  18. $this->error="Incorect nick or password";
  19. return false;
  20. }
  21. $this->isLoged=true;
  22. return true;


Wrócę jeszcze z pytaniem ponieważ było poruszone po co jest klasa Database. Czy rzeczywiście jest zbędna i powinienem w niej w konstruktorze tylko tworzyć połączenie?

Dzięki za rady. Dalej będę doszkalał się z coposera i z cs-fixera. Grzebie też pomału w linuxie, jednak miałem problem z localhostem i wyczytałem, że najlepiej w linuxie skorzystać z "kontenerów", jednak to zapowiada się grubszy temat. Przerobie wzorce, potem smarty i pewnie będę dalej coś tworzył.

Jak ktoś ma jeszcze jakieś uwagi to ja bardzo chętnie smile.gif

Nowe pliki pod tym samym linkiem.

Ten post edytował mrpickles 17.10.2019, 21:04:43
Go to the top of the page
+Quote Post
viking
post 18.10.2019, 07:24:15
Post #7





Grupa: Zarejestrowani
Postów: 5 397
Pomógł: 916
Dołączył: 30.08.2006

Ostrzeżenie: (0%)
-----


Przykładowo ta nieszczęsna klasa database. W przypadku błędu łapiesz wyjątek i w zasadzie nic z tym nie robisz. Czyli w sytuacji gdy wystąpił błąd mogę zrobić coś takiego:
  1. (new Database())->query();
$this->connection w query jest nullem - php sypie błędem.
Czemu każda klasa ma require 'vendor\autoload.php';?
Formy nie powinny rozszerzać Database bo i po co? Co najwyżej jakieś AbstractForm w którym można wstrzykiwać Connection (przykład bo raczej głupie rozwiązanie).
public function checkCredentials(array $data) :bool - credentials zazwyczaj będzie zawsze user, pass więc czemu nie public function checkCredentials(string $user, string $pass) :bool
Brakuje jakiegoś może service który obsługiwałby użytkowników. Przykładowo UserService -> checkCredentials i wtedy sprawdzasz $this->userService->check... zamiast wykonywać zapytania w klasie Form.
Smarty to niekoniecznie dobry pomysł. Raczej historycznie się go używa. Twig, Blade, PHPTAL, Volt.


--------------------
Go to the top of the page
+Quote Post
mrpickles
post 18.10.2019, 09:39:59
Post #8





Grupa: Zarejestrowani
Postów: 8
Pomógł: 0
Dołączył: 14.10.2019
Skąd: Białystok

Ostrzeżenie: (0%)
-----


Usunąłem vendor/autoload w klasach - pozostałość bo tworzeniu połączenia w konstruktorze poprzez new Database. Później zrobiłem Depenedncy injection, a require zostało - dzięki za zwrócenie uwagi.
Co to rozszerzania Form o Database - w tutorialu OOP który przerabiałem Dependency injection było pokazywane tylko na klasach które po sobie dziedziczą - uznałem to za obowiązkowe, a rzeczywiście nie jest i już usunięte.
Rozpisałem checkCredentials na przyjmowanie 2 stringów - w arrayu i tak siedział tam tylko user i password.
Dzięki za info o smartach, przyjrzę się pozostałym.

Możesz przybliżyć o co chodzi z userservice? Kompletnie nie wiem jak mogę to ugryźć.Chodzi o stworzenie nowej klasy userService i tam przeniesienie checkcredential? Tylko, że to główne zadanie klasy loginForm.

Zapytam przy okazji czy to jest prawidłowe, lepsze podejście to strony index w przypadku logowania? Uzależnienie wyświetlanej treści od tego czy jesteśmy zalogowani czy nie, czy lepsze jest przekierowywanie jak wykryje, ze jesteśmy zalogowani?

  1. <?php
  2.  
  3. $user = new User();
  4. if ($user->isLoggedIn()) {
  5. ?>
  6. <p>Hello <a href="profile.php?user=<?php echo escape($user->data()->username); ?>"><?php echo escape($user->data()->username); ?></a>!</p>
  7. <ul>
  8. <li><a href="update.php">Update Details</a></li>
  9. <li><a href="changepassword.php">Change Password</a></li>
  10. <li><a href="logout.php">Logout</a></li>
  11. </ul>
  12. <?php
  13. } else {
  14. echo "<p>You need to <a href='login.php'>login</a> or <a href='register.php'>register</a></p>";
  15. }
  16. ?>


Go to the top of the page
+Quote Post
athabus
post 18.10.2019, 10:02:48
Post #9





Grupa: Zarejestrowani
Postów: 893
Pomógł: 45
Dołączył: 2.11.2005
Skąd: Poznań

Ostrzeżenie: (0%)
-----


Ogólnie po zmianach wygląda już znacznie lepiej. Postaram się dać trochę uwag - to jeszcze nie wszystko co rzuciło mi się w oczy, ale nie chcę Cię wszystkim przytłaczać na raz.

1. GIT - w gicie masz coś takiego jak .gitignore pozwalające na nie commitowanie pewnych plików. Dla przykładu w 99% projektów nie komituje się vendorów, a już na pewno nie powinieneś commitować cachu od fixera, czy innych cachy/logów. Tu przy okazji będziesz miał fajne ćwiczenie jak przestać śledzić zmiany w repo czegoś co omyłkowo commitowałeś. No i zdecydowanie popracuj nad opisywaniem commitów - opis powinien być jasny i klarowny, a nie @Update. Jak będziesz kiedyś robił repo, które chcesz pokazać pracodawcy, to historia commitów jest często przeglądana. W firmach zazwyczaj pracuje się w systemach zadaniowych i opis kommita wygląda tak: [Task-abc] Opis taska.

2. PHP coś z tym fixerem poszło nie do końca ok, bo w wielu miejscach nadal masz kod niezgodny z PSR - np. $a=1 powinno być $a = 1; Ogólnie sporo masz tam jeszcze w kwestii PSR do poprawienia. Ale znów - jeśli myślisz o pracy zawodowo, to jest coś na co pracodawcy baaaardzo zwracają uwagę, więc wyrabiaj sobie nawyki.

3. Piszesz, że walczysz z diffem - są do tego narzędzia, które pokazują graficznie. Ja korzystam akurat z wbudowanego w PhpStorma, ale to płatne oprogramowanie, więc pewnie musisz poszukać jakiś alternatyw - może vscode ma podobne wtyczki, albo jakiś wyspecjalizowany edytor. W PphSotrm jest nawet opcja wpięcia fixera jako plugin, który sam poprawia PSR w pliku. Umiejętność pracy z Diffem jest kluczowa w pracy zespołowej, gdzie musisz sprawdzić co kto i kiedy zmieniał. Warto nad tym pracować.

4. Komentarze. Ogólnie tutaj szkoły są różne. Ja jestem zwolennikiem, że kod powinien się dokumentować sam, gdzie tylko to możliwe. Ogólnie komentarze powinny być stosowane gdy wnoszą coś nowego - jeśli są redundantne z metodą to są zbędne. Spójrz tutaj:
Kod
/*Row Count*/
    public function rowcount():int
    {
        return $this->stmt->rowcount();
    }

/**
* checking credentials
* @param array $argument1 array structure to verify credentials
* @return bool Return bool if credentials are correct
*/
  public function checkCredentials(array $data) :bool
{}


Te dwa komentarze nie wniosły NIC oprócz dodatkowego kodu, który trzeba teraz utrzymywać i poprawiać jak robisz refaktor metod.

Przydatne komentarze

Kod
    /**
     * @return OrderItemInterface[]
     * @throws NoSuchEntityException
     */
    protected function getItems(int $orderId): array
    {...    }


Tutaj komentarz coś wnosi bo informuje że w tablicy zwróconej przez metodę dostanę obiekty danego typu + że metoda może rzucić wyjątek danego typu. Zobacz, że już parametru nie ma w komentarzu, bo wszystko wynika z nagłówka metody.

5. Chyba już powoli warto zacząć się zastanawiać nad ubraniem tego w strukturę. Na początek poradziłbym Ci stworzyć wspólny entrypoint dla całej aplikacji. Pokombinuj jak to zrobić, aby wszystkei requestty uderzały w jeden punkt (index.php). W ten sposób wyeliminujesz potrzebę używania require w całej aplikacji. Na początek możesz to zrobić po prostu w formie frontcontrollera, choć docelowo pewnie raczej powinien to byś typowy plik inicjujący aplikację, a front controller już powinien być osobno. Niemniej na razie możesz to zrobić w 1 pliku i potem pomyśleć jak zrobić Refaktor.

6. Podoba mi się, że sam użyłeś strict_types - chyba o tym nie pisałem, a widzę że w klasach się pojawił.

7. Odnośnie pytania z ostatniego postu - łatwiej będzie to ogarnąć przekierowaniem na stronę z odpowiednim komunikatem bo ładnie obsłużych to na wyższym poziomie niż w szablonie.
Go to the top of the page
+Quote Post
viking
post 18.10.2019, 10:03:49
Post #10





Grupa: Zarejestrowani
Postów: 5 397
Pomógł: 916
Dołączył: 30.08.2006

Ostrzeżenie: (0%)
-----


https://martinfowler.com/eaaCatalog/
Dlaczego głównym zadaniem loginForm ma być sprawdzanie stanu użytkownika? Klasa powinna zawierać walidację, filtrowanie danych, ewentualnie zajmować się generowaniem inputów (na zasadzie buildera $loginForm->add('input', [params]).


--------------------
Go to the top of the page
+Quote Post
nospor
post 18.10.2019, 10:21:30
Post #11





Grupa: Moderatorzy
Postów: 34 792
Pomógł: 5789
Dołączył: 27.12.2004




Njapierw klasa dziedziczy po Database
class RegistrationForm extends Database

A potem ta sama klasa w konstruktorze dostaje obiekt Database
public function __construct(Database $connection)

Rozumiem, ze zapomniales usunac extednds Database ?


Klasa Database
  1. require_once('config\config.php');
  2. class Database
  3. {
  4. private $host=DB_HOST;
  5. private $user=DB_USER;
  6. private $password=DB_PASSWORD;
  7. private $dbname=DB_NAM

powinna byc niezalezna od jakiegos tam konfigu. Dane konfiguracyjne powinna dostac w konstruktorze

Klasa Database
  1. try {
  2. $this->connection= new PDO($dbh, $this->user, $this->password, $options);
  3. $this->isConnected =true;
  4. } catch (PDOException $e) {
  5. $this->error=$e->getmessage();
  6. $this->isConnected = false;
  7. }

Powinna raczej pluc wyjatkiem na zewnatrz w przypadku braku polaczenia a nie tlumic go wewnatrz siebie. Teraz tak to masz zrobione,ze pomimo braku polaczenia, inne klasa odpalaja zapytania na polaczeniu, ktorego moze teoretycznie nie byc

rowCount a nie rowcount
To samo dotyczy wywolan metody rowCount z PDO.

staraj sie czasami czytac co piszesz, np to:
$log->getIsLoged()
nie ma wiekszego sensu. Czy jak sie pytasz kogos, czy ma bulke to mowisz tak:
"Daj mi czy masz bulke"

Czy moze tak:
"Masz bulke?"

Tak samo tutaj powinno byc poprostu
$log->isLoged()

Juz nie wspomnie o braku jednego "g"

Powinienies miec jeden plik glowny/publiczny przez ktory przechodza wszystkie akcje, np index.php i on dopiero powinien odpalac poszczegolne akcje i inicjowac mase niezbednych rzeczy. Teraz masz 6 plikow glownych a w kazdym polowa kodu jest powtorzona

Naprawde nie musisz wszystkieg pchac w php. Np ten plik
  1. <?php
  2. if (!isset($_SESSION['loggedin'])) {
  3. header("location:index.php");
  4. }
  5. echo <<<END
  6. <a href="borrow.php"> Borrow book </a></br>
  7. <a href="return.php"> Return book </a></br>
  8. <a href="history.php"> Account history </a></br>
  9. <a href="logout.php"> Logout </a></br>
  10. END;


Moze wygladac tak:
  1. <?php
  2. if (!isset($_SESSION['loggedin'])) {
  3. header("location:index.php");
  4. }
  5. ?>
  6. <a href="borrow.php"> Borrow book </a></br>
  7. <a href="return.php"> Return book </a></br>
  8. <a href="history.php"> Account history </a></br>
  9. <a href="logout.php"> Logout </a>


--------------------

Najlepsze kawałki programistyczne || Dowcipy o informatykach || Forum PHP dla opornych
Klasy: Pager (stronicowanie) | Cache | ShoutBox (Chat) | Widok | Ładne url'e

"Myśl, myśl, myśl..." - Kubuś Puchatek || "Manual, manual, manual..." - Kubuś Programista
"Szukaj, szukaj, szukaj..." - Kubuś Odkrywca || "Debuguj, debuguj, debuguj..." - Kubuś Developer

Go to the top of the page
+Quote Post
athabus
post 18.10.2019, 10:36:44
Post #12





Grupa: Zarejestrowani
Postów: 893
Pomógł: 45
Dołączył: 2.11.2005
Skąd: Poznań

Ostrzeżenie: (0%)
-----


Jeszcze taka ogólna uwaga odnośnie dziedziczenia. Ogólnie dziedziczenie nie służy do "przekazywania" kodu do klas potomnych tak jak to robisz, czyli rozszerzając klasę o klasę Database. Dziedziczenie zazwyczaj to zło, chyba że robisz to świadomi i w przemyślany sposób. W większości przypadków lepsza od dziedziczenia jest kompozycja (wstrzyknięcie obiektu przez konstruktor). Jest to bardziej elastyczne rozwiązanie i nie blokuje rozwoju aplikacji. Dziedziczenie w prawdziwych aplikacjach używane jest dość rzadko - raczej stosuje się interfejsy + kompozycję. Na tak małym kodzie jak teraz piszesz może to być trudne do zrozumienia i wydawać się całkiem dobrym pomysłem, ale serio używaj dziedziczenia jak najrzadziej się da, bo przy większym kodzie to się szybko mści.
Go to the top of the page
+Quote Post
mrpickles
post 18.10.2019, 11:49:49
Post #13





Grupa: Zarejestrowani
Postów: 8
Pomógł: 0
Dołączył: 14.10.2019
Skąd: Białystok

Ostrzeżenie: (0%)
-----


Ustosunkuje się tylko niektórych elementów, bo dostałem dużo wskazówek, a zanim je przerobie minie dłuższa chwila.

1) Tak, nie wrzuciłem na githuba dokonanych w plikach zmian stąd w dalszym ciągu było widoczne extends - jak pisałem już tak było wskazane w tutorialu który przerobiłem dotyczący dependency injection, że można było je stosować tylko w przypadku dziedziczenia.Odniosłem po tym tutorialu wrażenie, że dziedziczenie jest bardzo ważne i szeroko stosowane, ale jak widać nie smile.gif
Jak coś dopiero teraz wrzuciłem zmienione pliki.

2) Jeśli chodzi o
  1. $log->getIsLoged()

tak samo z tutorialu wynikało jasno, że jeśli funkcja nam coś zwraca, powinniśmy ją nazywać od get - niby przyjęta praktyka. Rozumiem,że jednak nie za wszelką cenę, na pewno cenne info.

3) Cs-Fixer nadpisał mi wszystkie elementy w katalogu, sprawdzałem po dacie modyfikacji, specjalnie przestawiałem klamry w dziwne miejsca i poprawiał je po uruchomieniu.Być może kwestia, że wyświetla mi komunikat ze korzystam z vendora fabpot, a nie friendsofphp - mam obydwa w composerze, nie umiem jednak wymusic stosowania właśnie friendsofphp.

4) Chyba zaczynam rozumieć, o co chodzi z login formem i podziałem - chwilowo dla mnie to abstrakcja do napisania, ale poszukam powalczę.

5) Jeden z pierwszych postów mówił o usunięciu HTML, więc zrobiłem to wszędzie - brak mi zarówno wiedzy jak i praktyki, stąd usunąłem wszędzie, ale znowu widzę, że i tutaj mogę stosować zasadę nie za wszelką cenę smile.gif

6) co do komentarzy,trochę już mi się wyjaśniło, wydawało mi się to dziwne na przytoczonym przykładzie, też to odbierałem jako dublowanie kodu, myślałem,że tak poprostu ma być

Więc po kolei:

Jeśli chodzi o GITa ogarnę to, już wiem na co zwracać uwagę.

Potem ogarnę diffa, żeby zadbać o czystość kodu - zależy mi na nim.

Potem zaznajomię się z MVC i spróbuje przepuścić wszystko przez index i przebuduje Database, być może to mi rozjaśni wasze wskazówki.


Dzięki za odzew!

Go to the top of the page
+Quote Post
athabus
post 18.10.2019, 12:14:40
Post #14





Grupa: Zarejestrowani
Postów: 893
Pomógł: 45
Dołączył: 2.11.2005
Skąd: Poznań

Ostrzeżenie: (0%)
-----


To o czym piszesz to jest częsty błąd w tutorialach, bo prawdziwe OOP trudno ująć w krótkim tutorialu. Szybciej się go nauczysz pisząc jakąś apkę wspólnie z kimś kto temat ogarnia. Największe oszustwo OOP polega na tym, że ludzie próbują przenosić taksonomie z prawdziwego życia. Na przykład typowy przykład - mam psa i rybkę -> stworzę klasę Animal. To jest w 80% tutoriali o OOP, a jest to anty przykład jak stosować OOP. Tak jak pisałem dzisiaj będzie Ci to trudno zrozumieć bo piszesz zbyt mały kod, ale w prawdziwym kodzie to jest droga do wielkiej katastrofy.
Wpisz w Google jedną z podstawowych zasad OOP "Favor composition over inheritance" - na razie nie zaprzątaj sobie tym głowy bo w Twoim kodzie są większe problemy niż stosowanie SOLID, ale ogólnie dziedziczenia staraj się unikać to zaprocentuje w przyszłości.

* tu taki disclaimer - ja nie jetem przeciwnikiem dziedziczenia. W wielu miejscach jest bardzo przydatne - np. metoda szablonowa etc. Ale na pewno nie w takiej formie jak uczą go w tutorialach dla początkujących. To tylko tworzenie złych nawyków, które potem trudno zwalczyć, a kompozycja jest bardzo łatwym konceptem do ogarnięcia więc można od razu wyrabiać dobre wzorce.

Tymczasem do działa - czekam na kolejną wersję kodu. Jeszcze z 20 -30 iteracji i będzie dobrze ;-)

PS. z tym HTML to chyba źle zrozumiałeś intencje kolegi - chodziło mu o zupełne wydzielenie widoku z logiki, a nie zamianę kodu html na stringi w PHP. Jak poczytasz o MVC to zobaczysz o co chodzi. Nie musisz tu używać TWIGA/SMARTY - widok możesz zrobić na prostym PHP + HTML, ale nie mieszaj widoku z logiką.
Go to the top of the page
+Quote Post
nospor
post 18.10.2019, 12:15:38
Post #15





Grupa: Moderatorzy
Postów: 34 792
Pomógł: 5789
Dołączył: 27.12.2004




Cytat(mrpickles @ 18.10.2019, 11:49:49 ) *

Jak extends bylo tak nadal jest

Cytat
2) Jeśli chodzi o
  1. $log->getIsLoged()

tak samo z tutorialu wynikało jasno, że jeśli funkcja nam coś zwraca, powinniśmy ją nazywać od get - niby przyjęta praktyka. Rozumiem,że jednak nie za wszelką cenę, na pewno cenne info.
Tak, ale to dotyczy rzeczownikow a nie pytan. isLogged to pytanie samow sobie wink.gif


Cytat
5) Jeden z pierwszych postów mówił o usunięciu HTML, więc zrobiłem to wszędzie - brak mi zarówno wiedzy jak i praktyki, stąd usunąłem wszędzie, ale znowu widzę, że i tutaj mogę stosować zasadę nie za wszelką cenę smile.gif
CHodzilo o uzycie plikow szablonow a nie wstawianie tego w echo jak masz teraz. Skoro nie uzywasz plikow szablonow poki co, to nie ma sensu calego duzego html pchac bez sensu w echo



--------------------

Najlepsze kawałki programistyczne || Dowcipy o informatykach || Forum PHP dla opornych
Klasy: Pager (stronicowanie) | Cache | ShoutBox (Chat) | Widok | Ładne url'e

"Myśl, myśl, myśl..." - Kubuś Puchatek || "Manual, manual, manual..." - Kubuś Programista
"Szukaj, szukaj, szukaj..." - Kubuś Odkrywca || "Debuguj, debuguj, debuguj..." - Kubuś Developer

Go to the top of the page
+Quote Post
mrpickles
post 18.10.2019, 13:12:23
Post #16





Grupa: Zarejestrowani
Postów: 8
Pomógł: 0
Dołączył: 14.10.2019
Skąd: Białystok

Ostrzeżenie: (0%)
-----


Zakręciłem się i pracowałem na klonie, a zrobiłem upload nie klona z jakimiś innymi drobnymi poprawkami. Już na pewno nie ma extends.

Nie byłem pewny czy chodzi o wyechowanie htmla, czy właśnie jak pytałem o zrobienie np. index do którego includuje index_view, który to zawiera html - ale miałem problem żeby oddzielić to w 100%.

Korzystając z okazji zanim będę przerabiał temat to dopytam właśnie o to.

Oddzielenie html od php na przykładzie z dużym echem - chodzi mi o sam sposób, a nie czy w tym pliku to rzeczywiście było potrzebne smile.gif
mysite
  1. <?php
  2. if (!isset($_SESSION['loggedin'])) {
  3. header("location:index.php");
  4. }
  5. require("mysite_view.php");

mysite_view
  1. <a href="borrow.php"> Borrow book </a></br>
  2. <a href="return.php"> Return book </a></br>
  3. <a href="history.php"> Account history </a></br>
  4. <a href="logout.php"> Logout </a></br>
  5.  


Jednak w przypadku return wydaje mi się to niemożliwe z uwagi na wtopienie w html php w petli foreach.
  1. <?php
  2.  
  3. if (!isset($_SESSION['loggedin'])) {
  4. header("location:index.php");
  5. }
  6.  
  7. require __DIR__ . '/vendor/autoload.php';
  8. use \Library\Library;
  9. use \Library\Database;
  10.  
  11. $db = new Database();
  12. $book = new Library($db);
  13. if (isset($_POST['return'])) {
  14. $book->returnBook($_POST);
  15. }
  16. echo '<form name="return" method="post">';
  17.  
  18. foreach ($book->getBorrowedBooks() as $position) {
  19. echo $position->autor.$position->tytul.$position->rok."<input type='checkbox' name='return[]' value=".$position->idksiazki."></br>";
  20. }
  21. echo '<input type="submit" value="Return"></form><a href="mysite.php">Back</a>';


Czy chodzi w takim przypadku o nierodzielanie do dwóch plików, a o zastosowanie składni alternatywnej dla php? Czy jednak chodzi o fizyczne rozdzielanie?
Go to the top of the page
+Quote Post
nospor
post 18.10.2019, 13:33:05
Post #17





Grupa: Moderatorzy
Postów: 34 792
Pomógł: 5789
Dołączył: 27.12.2004




Generalnie nie chodzi o odzielenie php od html a o odzielenie logiki aplikacji od wyswietlania. W logice aplikacji przygotowujesz wszystkie dane, ktore potem przekazujesz do widoku ktory to te dane wysweitla.
A czy widoko zrealizujesz w php czy za pomoca systemu szablonow to juz inna sprawa.


--------------------

Najlepsze kawałki programistyczne || Dowcipy o informatykach || Forum PHP dla opornych
Klasy: Pager (stronicowanie) | Cache | ShoutBox (Chat) | Widok | Ładne url'e

"Myśl, myśl, myśl..." - Kubuś Puchatek || "Manual, manual, manual..." - Kubuś Programista
"Szukaj, szukaj, szukaj..." - Kubuś Odkrywca || "Debuguj, debuguj, debuguj..." - Kubuś Developer

Go to the top of the page
+Quote Post
mrpickles
post 18.10.2019, 13:37:07
Post #18





Grupa: Zarejestrowani
Postów: 8
Pomógł: 0
Dołączył: 14.10.2019
Skąd: Białystok

Ostrzeżenie: (0%)
-----


Ok czyli MVC się kłania. Dzięki za wyjaśnienia, " I'll be back " smile.gif
Go to the top of the page
+Quote Post
phpion
post 3.11.2019, 18:59:27
Post #19





Grupa: Moderatorzy
Postów: 6 065
Pomógł: 857
Dołączył: 10.12.2003
Skąd: Dąbrowa Górnicza




Wiele już zostało powiedziane ale ja dodam od siebie jedno, na co nikt wcześniej nie zwrócił uwagi. Chodzi o:
https://github.com/filipdak/libraryoop/blob...onForm.php#L103
  1. $this->connection->query("INSERT INTO uzytkownicy VALUES(NULL,:nick,:email,:password)");

Unikaj zapisów poleceń SQL bazujących na kolejności kolumn w bazie danych. Zmienisz strukturę tabeli, dodasz pomiędzy istniejącymi kolumnami nową i zaczną się problemy z dziwnym zachowaniem skryptu. Dużo lepiej i bezpieczniej jest zapisać (w tym pomijając NULL dla kolumny id):
  1. $this->connection->query("INSERT INTO uzytkownicy (nick, email, password) VALUES(:nick,:email,:password)");

czyli jawnie podać jakie dane mają trafić w jakie kolumny.
Go to the top of the page
+Quote Post

Reply to this topicStart new topic
1 Użytkowników czyta ten temat (1 Gości i 0 Anonimowych użytkowników)
0 Zarejestrowanych:

 



RSS Wersja Lo-Fi Aktualny czas: 19.11.2019 - 16:56