Wydajność funkcji PHP

Witam
W ramach praktyki tworzę przeglądarkową grę transportową i mam pytanie odnośnie wydajności jednej z funkcji

Oto ona:

public function SprawdzCzyWykonanoZlecenie()
{
  //poniżej sprawdzam czy czas zlecenia danego użytkownika minął tzn czy zlecenie zostało wykonane
  $SprawdzCzyWykonanoZlecenie = DB::select("SELECT * FROM zlecenia, ladunki, users WHERE zlecenia.czastrwaniazleceniaz <= ? AND zlecenia.iduzytkownikz = ? AND users.id = ?",[time(),Auth::id(),Auth::id()]);
  $IdLaczenia = [];
  //niestety w tej pętli są dwa zapytania do bazy danych
  foreach($SprawdzCzyWykonanoZlecenie as $WykonaneZlecenie)
  {
      //saldo po dodaniu zaplaty za wykonane zlecenie
      $Saldo = $WykonaneZlecenie->zaplataladunek + $WykonaneZlecenie->waluta;
      //aktualizacja salda po otrzymaniu zapłaty za wykonane zlecenie
      DB::update("UPDATE users SET waluta = ? WHERE id = ?",[$Saldo,Auth::id()]);
      //usunięcie wykonanego zlecenia - jest ono już niepotrzebne
      DB::delete("DELETE FROM zlecenia WHERE zlecenia.idz = ?",[$WykonaneZlecenie->idz]);
  }

Pytanie brzmi:
Powyższy kod ma trzy zapytania do bazy z czego dwa moga się wykonać wiele razy - czy jest to odpowiednie rozwiązanie czy powinienem rozważyć inne rozwiązanie tego problemu.
Zaznaczę że powyższa funkcja wywoływana jest podczas wejścia na podgląd zleceń.
Szczerze mnie te zapytania do bazy danych rażą w oczy, czy słusznie ?
Za pomoc z góry dziękuję
Pozdrawiam

Kod wklejaj przez markdown czyli ```php.
Zacznijmy od początku. Zmienne w PHP zaczynasz od małej litery.
https://www.php-fig.org/psr/psr-12/

Po tym kodzie sądzę że korzystasz z jakiegoś frameworka, Laravel? Czemu nie korzystasz z jego możliwości?
Jak już robisz select, staraj się nie używać gwiazdki. Pobieraj pola jakich potrzebujesz. Pobieranie wszystkich danych jest zła praktyką.
Myślę, że można to zastąpić jakimś grupowaniem.
Czy np. to usuwanie zleceń jest w ogóle dobre z perspektywy apliakcji? Czy nie lepiej ustawić flagę "wykonane"? Co jak będziesz miał np. zwrócić częściowo płatność etc?
Pomijając powyższe taki delete możesz zrobić poprzez jedną operacje na bazie z użyciem id np. where in() albo odpowiedni warunek innego typu, w takim wypadku warto pomyśleć o ustawieniu transakcji na całe operacje bazodanowe.

ktosCZYLIJa napisał(a):

Witam
W ramach praktyki tworzę przeglądarkową grę transportową i mam pytanie odnośnie wydajności jednej z funkcji

Oto ona:

Kopiuj

public function SprawdzCzyWykonanoZlecenie()
{
  //poniżej sprawdzam czy czas zlecenia danego użytkownika minął tzn czy zlecenie zostało wykonane
  $SprawdzCzyWykonanoZlecenie = DB::select("SELECT * FROM zlecenia, ladunki, users WHERE zlecenia.czastrwaniazleceniaz <= ? AND zlecenia.iduzytkownikz = ? AND users.id = ?",[time(),Auth::id(),Auth::id()]);
  $IdLaczenia = [];
  //niestety w tej pętli są dwa zapytania do bazy danych
  foreach($SprawdzCzyWykonanoZlecenie as $WykonaneZlecenie)
  {
      //saldo po dodaniu zaplaty za wykonane zlecenie
      $Saldo = $WykonaneZlecenie->zaplataladunek + $WykonaneZlecenie->waluta;
      //aktualizacja salda po otrzymaniu zapłaty za wykonane zlecenie
      DB::update("UPDATE users SET waluta = ? WHERE id = ?",[$Saldo,Auth::id()]);
      //usunięcie wykonanego zlecenia - jest ono już niepotrzebne
      DB::delete("DELETE FROM zlecenia WHERE zlecenia.idz = ?",[$WykonaneZlecenie->idz]);
  }

Już pomijając słabe nazwy i te nadmiarowe zapytania, to ogólnie ten kod jest raczej niskiej jakości. Moim zdaniem największa wada tej funkcji to słaby design, bo nie widać w ogóle o co w niej chodzi, logika pomieszana z sqlem, tight-coupling na framework, etc.

Dziękuję za cenne uwagi.
Zmieniłem swoją funkcję na taką:

public function SprawdzCzyWykonanoZlecenie()
    {
        $sprawdzCzyWykonanoZlecenie = DB::table('zlecenia')->leftJoin('users','users.id','=','zlecenia.iduzytkownikz')->leftJoin('ladunki','zlecenia.idladunekz','=','ladunki.idladunek')->where('users.id',Auth::id())->where('zlecenia.iduzytkownikz',Auth::id())->where("zlecenia.czastrwaniazleceniaz",'<=',time())->select("waluta","idz","zaplataladunek")->get();
        $idLaczenia = [];
        foreach($sprawdzCzyWykonanoZlecenie as $wykonaneZlecenie)
        {
            $saldo = $wykonaneZlecenie->zaplataladunek + $wykonaneZlecenie->waluta;
            DB::beginTransaction();
            DB::table("users")->where('id',Auth::id())->update(['waluta' => $saldo]);
            DB::table("zlecenia")->where('zlecenia.idz',$wykonaneZlecenie->idz)->update(['zlecenia.czywykonano' => true]);
            DB::commit();

        }
    }

Nie wiem czy w transakcji (pomiędzy 'beginTransaction' i 'commit') mogę mieć oprócz zapytań do bazy inne elementy (na przykład przypisanie wartości do zmiennej).

Możesz mieć dowolny kod powiązany z tymi operacjami w tym jakieś zmienne. Nie ma problemu.
Kod ma wykonywać zadania i być czytelny. Pamiętaj więc o tym, żeby kod był czytelny.
Jeśli Twoja funkcja już robi co należy, zastanów się nad zmianą jej nazwy. Nazwa zaczynająca się od Sprawdź sugeruje zwrócenie jakiegoś wyniku typu bool.
Tu nie zwracasz nic. Funkcja może nazywać się np. ZaktualizujStatusZamowienUzytkownika()

ktosCZYLIJa napisał(a):

Nie wiem czy w transakcji (pomiędzy 'beginTransaction' i 'commit') mogę mieć oprócz zapytań do bazy inne elementy (na przykład przypisanie wartości do zmiennej).

a dlaczego transakcja nie obejmuje całego foreacha?
masz tam jeszcze jakiś kod którego tu ie widać?
a zmienna idLaczenia do czego jest?

a dlaczego transakcja nie obejmuje całego foreacha?

Podejrzewam, że tych elementów w foriczu może być tyle, że jest ryzyko, iż transakcja skutecznie zatka bazę na X minut.

ktosCZYLIJa napisał(a):

Dziękuję za cenne uwagi.
Zmieniłem swoją funkcję na taką:

Kopiuj

public function SprawdzCzyWykonanoZlecenie()
    {
        $sprawdzCzyWykonanoZlecenie = DB::table('zlecenia')->leftJoin('users','users.id','=','zlecenia.iduzytkownikz')->leftJoin('ladunki','zlecenia.idladunekz','=','ladunki.idladunek')->where('users.id',Auth::id())->where('zlecenia.iduzytkownikz',Auth::id())->where("zlecenia.czastrwaniazleceniaz",'<=',time())->select("waluta","idz","zaplataladunek")->get();
        $idLaczenia = [];
        foreach($sprawdzCzyWykonanoZlecenie as $wykonaneZlecenie)
        {
            $saldo = $wykonaneZlecenie->zaplataladunek + $wykonaneZlecenie->waluta;
            DB::beginTransaction();
            DB::table("users")->where('id',Auth::id())->update(['waluta' => $saldo]);
            DB::table("zlecenia")->where('zlecenia.idz',$wykonaneZlecenie->idz)->update(['zlecenia.czywykonano' => true]);
            DB::commit();

        }
    }

Nie wiem czy w transakcji (pomiędzy 'beginTransaction' i 'commit') mogę mieć oprócz zapytań do bazy inne elementy (na przykład przypisanie wartości do zmiennej).

1. Używaj nazw angielskich do wszystkiego. Nie tylko nazwy funkcji i zmiennych w php, ale też w bazie danych. Raz masz tabelę users po angielsku, za chwilę masz 'zlecenia'.

2. Nazwy czegokolwiek mają wskazywać czym to tak naprawdę jest. Zmienna $sprawdzCzyWykonanoZlecenie nie sprawdza, a pobiera zlecenia. Kolumna w bazie danych waluta, nie jest walutą a wartością liczbową, na której później wykonujesz obliczenia.

3. Jeśli robisz to w ramach nauki, nie przejmuj się, że popełniasz błędy. W nauce programowania nie chodzi o to, by napisać coś byle by działało. Czytaj uwagi, poprawiaj kod i staraj się nie popełniać później tych samych błędów. Nauka to proces, zwłaszcza programowania.

Możesz też wykorzystać chatgpt na początku nauki do code review. Po prostu wklej kod i zapytaj co można w tym kodzie poprawić.

ktosCZYLIJa napisał(a):

Dziękuję za cenne uwagi.
Zmieniłem swoją funkcję na taką:

Kopiuj

public function SprawdzCzyWykonanoZlecenie()
    {
        $sprawdzCzyWykonanoZlecenie = DB::table('zlecenia')->leftJoin('users','users.id','=','zlecenia.iduzytkownikz')->leftJoin('ladunki','zlecenia.idladunekz','=','ladunki.idladunek')->where('users.id',Auth::id())->where('zlecenia.iduzytkownikz',Auth::id())->where("zlecenia.czastrwaniazleceniaz",'<=',time())->select("waluta","idz","zaplataladunek")->get();
        $idLaczenia = [];
        foreach($sprawdzCzyWykonanoZlecenie as $wykonaneZlecenie)
        {
            $saldo = $wykonaneZlecenie->zaplataladunek + $wykonaneZlecenie->waluta;
            DB::beginTransaction();
            DB::table("users")->where('id',Auth::id())->update(['waluta' => $saldo]);
            DB::table("zlecenia")->where('zlecenia.idz',$wykonaneZlecenie->idz)->update(['zlecenia.czywykonano' => true]);
            DB::commit();

        }
    }

Nie wiem czy w transakcji (pomiędzy 'beginTransaction' i 'commit') mogę mieć oprócz zapytań do bazy inne elementy (na przykład przypisanie wartości do zmiennej).

Lepiej po prostu opisz co ta funkcja ma robić dokładnie. Nie implementację jaką napisałeś, tylko jaki jest cel tej funkcji? Czy ona ma zsumować całkowitą historie wymian czy coś takiego?

Funkcja ma za zadanie sprawdzić czy zlecenie (które zostało wzięte przez użytkownika) zostało wykonane, następnie jeśli tak jest to doliczana jest zapłata za wykonane zlecenie do konta (wirtualne pieniądze), a następnie dokonywana jest aktualizacja zlecenia aby przyjęło status wykonane i potem nie będzie się wyświetlać na liście aktualnie wziętych zleceń zalogowanego użytkownika

ktosCZYLIJa napisał(a):

Funkcja ma za zadanie sprawdzić czy zlecenie (które zostało wzięte przez użytkownika) zostało wykonane, następnie jeśli tak jest to doliczana jest zapłata za wykonane zlecenie do konta (wirtualne pieniądze), a następnie dokonywana jest aktualizacja zlecenia aby przyjęło status wykonane i potem nie będzie się wyświetlać na liście aktualnie wziętych zleceń zalogowanego użytkownika

no i i nadal nie wiem co to jest to
$IdLaczenia = []
a tą funkcję warto by w całości w bazie danych napisać ;)

Przepraszam zmienna tablicowa $IdLaczenia to jest mój błąd - nie powinno jej tam być.
Przepraszam bo nie ogarniam: o co chodzi z tym zapisem w bazie danych tej funkcji ? Chyba chodzi o sam kod sql zamiast tych 3 zapytań???

To może inaczej.
Wklej kod z komentarzami, co jaka linia robi i jaki masz zamiar.
Np.

// Chce tu pobrać dane bo muszę przeliczać jakieś operacje walutowe na kolumnach z,u,z w tabeli A
SprawdzCzyWykonanoZle{
            $saldo = $wykonaneZlecenie->zaplataladunek + $wykonaneZlecenie->waluta;
// Zaczynam transakcje tutaj bo chciałbym aby zachować spójność zapisu pojedyńcze zlecneia
DB::beginTransaction();
            DB::table("users")->where('id',Auth::id())->update(['waluta' => $saldo]);
            DB::table("zlecenia")->where('zlecenia.idz',$wykonaneZlecenie->idz)->update(['zlecenia.czywykonano' => true]);
            DB::commit();

        }
    }

jeszcze jedno update salda mogłoby być na zewnątrz foreach, po jej zakończeniu, w trakcie jednej i tej samej transakcji.
w foreach byś sobie zliczał sumę w zmiennej php
czy to update nie powinno też dodać początkowej wartości waluty z tabeli users?

ktosCZYLIJa napisał(a):

Zaznaczę że powyższa funkcja wywoływana jest podczas wejścia na podgląd zleceń.

Po pierwsze to dlaczego pole z saldem nazywa się "waluta", takie mieszanie pojęć szybko prowadzi do zamieszania w kodzie i bólu głowy przy próbie zrozumienia co się dzieje.
Po drugie i najważniejsze - twój kod jest podatny na race condition attack / double spending, wystarczy że "atakujący" / przebiegły gracz wyśle kilkanaście takich samych requestów na raz i jest duże prawdopodobieństwo że więcej niż jedno zapytanie jednocześnie wykona SELECT zanim dojdzie do UPDATE tym samym podwajając sobie zarobioną kasę. Ten błąd jest bardzo popularny i na wielu stronach wystarczy nawet szybkie dwukrotne kliknięcie na przycisk żeby wykonać jednorazową operację dwukrotnie, jak np zrealizowanie kodu rabatowego; co najmniej trzykrotnie zdarzyło się to nawet z giełdami crypto gdzie userzy mogli podwajać w ten sposób swoje wypłaty.

Większość baz danych pozwala na UPDATE z SELECTem z wielu tabel jednocześnie robiąc z tego atomowe zapytanie - lub SELECT FOR UPDATE w transakcji żeby zablokować wybrane wiersze.
Tak naprawdę prawdopodobnie w ogóle nie potrzebujesz tu swojego foreacha, a wszystko można załatwić jednym UPDATE.
Możesz też zrobić trigger który / constraint który wykrzaczy się przy próbie odznaczenia wykorzystanej transakcji przez updatem salda i zmianą kolejności tak żeby transakcja była odznaczana jako zrealizowana PRZED updatem salda - wtedy jeśli dwa wątki jednocześnie wskoczą w to miejsce to jeden z nich wywali się z błędem przed błędnym podwojeniem salda użytkownika.

Można to też rozwiązać przez kolejkowanie zdarzeń tak żeby tylko jeden wątek był odpowiedzialny za zarządzanie transakcjami i wykonywanie tych update'ów nie podczas wchodzenia na stronę ale w workerze na serwerze / cron jobie. Polecam rozwiązanie z kolejkowaniem bo można wtedy kod pisać "na spokojnie" nie blokując tabel transakcjami ani nie martwiąc się wszystkimi możliwymi scenariuszami równoległości wykonywania kodu. Taki kod jednowątkowy jest łatwy do zrozumienia, debugowania i trudny do zepsucia.

No i w końcu po trzecie - twój kod ma błąd, wybierasz aktualne saldo użytkownika i wartość transakcji a potem liczysz nowe saldo dodając te dwie wartości. To będzie działać dobrze dla jednej transakcji ale jeśli ich będzie więcej to saldo będzie takie samo i końcowa wartość salda będzie wynikać tylko i wyłącznie z ostatniej transakcji zamiast sumować wszystkie.

obscurity napisał(a):

ktosCZYLIJa napisał(a):

Zaznaczę że powyższa funkcja wywoływana jest podczas wejścia na podgląd zleceń.

Po pierwsze to dlaczego pole z saldem nazywa się "waluta", takie mieszanie pojęć szybko prowadzi do zamieszania w kodzie i bólu głowy przy próbie zrozumienia co się dzieje.
Po drugie i najważniejsze - twój kod jest podatny na race condition attack / double spending, wystarczy że "atakujący" / przebiegły gracz wyśle kilkanaście takich samych requestów na raz i jest duże prawdopodobieństwo że więcej niż jedno zapytanie jednocześnie wykona SELECT zanim dojdzie do UPDATE tym samym podwajając sobie zarobioną kasę. Ten błąd jest bardzo popularny i na wielu stronach wystarczy nawet szybkie dwukrotne kliknięcie na przycisk żeby wykonać jednorazową operację dwukrotnie, jak np zrealizowanie kodu rabatowego; co najmniej trzykrotnie zdarzyło się to nawet z giełdami crypto gdzie userzy mogli podwajać w ten sposób swoje wypłaty.

tu chyba nie, no zauważ że o robi update na podstawie aktualnego wpisu w bazie i dwa razy tego wpisu w bazie nie obsłuży nawet jak ktoś dwa razy kliknie
ale ogólnie moz lepiej takiego update nie robić z sumowaniem w php tylko w bazie kazać zrobić +=
DB::update("UPDATE users SET waluta = waluta + ? WHERE id = ?",[$Saldo,Auth::id()]);

ktosCZYLIJa napisał(a):

Kopiuj

//chcę sprawdzić czy zlecenia zostały wykonane, dokonać zapłaty za wykonane zlecenie dla użytkownika (tabela users)
//i ustawić kolumnę czywykonano na true co będzie znaczyć że zlecenie jest już wykonane i nie będzie się wyświetlało na //stronie aktualnych zleceń
public function SprawdzCzyWykonanoZlecenie()
    {
//Porównuje identyfikator użytkownika z zapisem jego identyfikatora w tabeli zlecenia,
//co znaczy że mam dostęp do zleceń aktualnie zalogowanego użytkownika
//sprawdzam czy czas obecny jest większy niż czas zakończenia zlecenia  (używam znacznika czasu unixa - time()
//jeżeli czas obecny jest większy niż czas zakończenia tzn że zlecenie zostało wykonane
                $sprawdzCzyWykonanoZlecenie = DB::table('zlecenia')->leftJoin('users','users.id','=','zlecenia.iduzytkownikz')->leftJoin('ladunki','zlecenia.idladunekz','=','ladunki.idladunek')->where('users.id',Auth::id())->where('zlecenia.iduzytkownikz',Auth::id())->where("zlecenia.czastrwaniazleceniaz",'<=',time())->select("waluta","idz","zaplataladunek")->get();

//początek transakcji
            DB::beginTransaction();

//przeszukuje zlecenia które zostały wykonane, (jeśli takich nie ma pętla się nie wykona)
        foreach($sprawdzCzyWykonanoZlecenie as $wykonaneZlecenie)
        {
//ustalam saldo dodania zapłaty za zlecenie do obecnego stanu konta użytkownika
            $saldo = $wykonaneZlecenie->zaplataladunek + $wykonaneZlecenie->waluta;
//zmieniam wartość wirtualnej waluty z obecnej na saldo zapisane w zmiennej $saldo
            DB::table("users")->where('id',Auth::id())->update(['waluta' => $saldo]);
//ustawiam kolumne tabeli czywykonano na true co jest róœnoważne z tym że zlecenie zostało wykonane 
            DB::table("zlecenia")->where('zlecenia.idz',$wykonaneZlecenie->idz)->update(['zlecenia.czywykonano' => true]);

        }
//wykonanie transakcji
            DB::commit();

    }

Twój opis w komentarzach ma bardzo dużo szczegółów implementacyjnych, ale ma też na tyle informacji o tym co funkcja ma robić, że udało mi się we wmiare sensowny sposób opisać założenia tej funkcji:

<?php

// chcę:
// dokonać zapłaty za wykonane zlecenie dla użytkownika
//   - czyli zwiększyć saldo użytkownika, o wynagrodzenie zlecenia
// usunąć zlecenie z aktualnych zleceń (impl: ustawić kolumnę czywykonano na true co będzie znaczyć że zlecenie nie będzie się wyświetlało na stronie aktualnych zleceń)

function SprawdzCzyWykonanoZlecenie() {

    // biorę pod uwagę tylko zlecenia aktualnie zalogowanego użytkownika -> (impl: porównując identyfikator użytkownika z zapisem jego identyfikatora w tabeli zlecenia)
    // listuję wykonane zadania -> (impl: jeżeli czas obecny jest większy niż czas zakończenia)

    $sprawdzCzyWykonanoZlecenie = DB::table('zlecenia')
        ->select("waluta", "idz", "zaplataladunek")
        ->leftJoin('users', 'users.id', '=', 'zlecenia.iduzytkownikz')
        ->leftJoin('ladunki', 'zlecenia.idladunekz', '=', 'ladunki.idladunek')
        ->where('users.id', Auth::id())
        ->where('zlecenia.iduzytkownikz', Auth::id())
        ->where("zlecenia.czastrwaniazleceniaz", '<=', time())
        ->get();

    DB::beginTransaction();
    // przeszukuje zlecenia które zostały wykonane (impl: pętla)
    foreach ($sprawdzCzyWykonanoZlecenie as $wykonaneZlecenie) {
        // Dodaję wynagrodzenie do salda użytkownika    
        // (impl: ustalam saldo dodania zapłaty za zlecenie do obecnego stanu konta użytkownika
        // zmieniam wartość wirtualnej waluty z obecnej na saldo)
        DB::table("users")->where('id', Auth::id())->update([
          'waluta' => $wykonaneZlecenie->zaplataladunek + $wykonaneZlecenie->waluta
        ]);
        // ustawiam że zlecenie zostało wykonane (impl: ustawiam kolumne tabeli czywykonano na true) 
        DB::table("zlecenia")->where('zlecenia.idz', $wykonaneZlecenie->idz)->update(['zlecenia.czywykonano' => true]);
    }
    DB::commit();
}

Czyli jeśli dobrze rozumiem, to odszyfrowując Twoją tajemniczą funkcję, to co chcesz zrobić to tak na prawdę, w momencie kiedy zlecenie jest wykonane, chcesz usunąć je z aktualnych zleceń i zwiększyć saldo użytkownika o podane wynagrodzenie.

Dziękuję Ci Riddle.
Wlaśnie takiego czegoś mi brakowalo, to jest aby ktoś mi pokazal jak to ma wyglądać.
Niestety nie mam się kogo zapytać w wielu kwestiach w programowaniui dlatego postanowilem się udzielić na tym forum.
Nie spodziewalem się takiego zainteresowania tematem, za które Bardzo Dziękuję
Pozdrawiam

ktosCZYLIJa napisał(a):

Dziękuję Ci Riddle.
Wlaśnie takiego czegoś mi brakowalo, to jest aby ktoś mi pokazal jak to ma wyglądać.
Niestety nie mam się kogo zapytać w wielu kwestiach w programowaniui dlatego postanowilem się udzielić na tym forum.
Nie spodziewalem się takiego zainteresowania tematem, za które Bardzo Dziękuję

Moim zdaniem rzeczy które są źle w tym kodzie to tak:

• Najważniejsze:
  • Przeczytaj książkę "Clean Code", w szczególności fragment o samoopisującym się kodzie. Z Twojej funkcji w ogóle nie wiadomo co robi.
  • Pomieszane wszystko z wszystkim: logika, baza, autentykacja w jednym miejscu. Chociaż byś przekazał time() oraz id zalogowanego usera przez parametr.
• Średnio ważne:
  • Przez to że robisz osobno selecta i update'a, to taki kod działą rzetelnie tylko wtedy kiedy wykonuje się jeden na raz, czyli synchronicznie. Gdybyś miał dwa procesy jednocześnie odpalające tą funkcje, to wystąpi race condition, w zalezności od kolejności kiedy który proces odpali select/update/delete. Transkacje tutaj nie pomogę, należałoby tutaj albo poprawić call do bazy tak żeby zrobił to jednym zapytaniem, albo użyć SELECT FOR UPDATE.
• Szczegóły:
  • Polskie nazwy w zmiennych i w bazie. Popraw na angielskie.

@ktosCZYLIJa A jeśli chciałbyś wiedzieć, jak moim zdaniem należałoby się zabrać za coś takiego, to ja by proponował takie podejście:

1. Sprecyzuj co chcesz żeby program robił, w języku który ogarnia użytkownik tej apki. W twoim przypadku chodzi o to, żeby user otrzymał wynagrodzenie za zlecenie w momencie jego zakończenia i żeby zakończone zlecenie zniknęło z listy aktualnych zleceń.
2. Przygotuj przykład takiego zachowania. Potrzebujesz dwóch przykładów, bo masz dwa zachowania.
   Kopiuj

   # Przykład 1
   - Jest użytkownik "Jan Kowalski" z kwotą 50zł na koncie
   - Dodaję zlecenie na 100zł do 21 czerwca dla "Jan Kowalski"
   - Nadchodzi 21 czerwca
   - Użytkownik "Jan Kowalski" powinien mieć teraz 150zł na koncie
   
   # Przykład 2
   - Dodaję zlecenie do 21 czerwca
   - Zlecenie jest w liście aktualnych ogłoszeń
   - Nadchodzi 21 czerwca
   - Zlecenie nie jest widoczne w liście aktualnych ogłoszeń
   

3. Przepisz ten przykład jako wykonywalną specyfikację (również znaną jako testy ) w dowolnej technologii. Możesz to zrobić phpunit, w behacie, albo w czymś całkiem zewnętrznym jak np. cypress. Przykładowe testy mogłyby wyglądać tak:
   Kopiuj

   #[Test]
   public function salaryIsTransferedToUserBalance_whenErrandIsFinished() {
     $this->addUser('Jan Kowalski', 50);
     $this->addErrand(100, '21-06-2025', 'Jan Kowalski');
     $this->setCurrentDate('21-06-2025');
     $this->assertEquals(150, $this->getUserBalance('Jan Kowalski'));
   }
   
   #[Test]
   public function errandIsListedAsAvailable_givenErrandIsNotFinished() {
     $this->addErrand('Roof work', 100, '21-06-2025');
     $this->setCurrentDate('20-06-2025');
     $this->assertEquals(true, $this->getErrandAvailable('Roof work'));
   }
   
   #[Test]
   public function errandIsNoLongerAvailable_whenErrandIsFinished() {
     $this->addErrand('Roof work', 100, '21-06-2025');
     $this->setCurrentDate('21-06-2025');
     $this->assertEquals(false, $this->getErrandAvailable('Roof work'));
   }
   

Specyfikację tzn czy mam to opisać jako dokumentację?
Przepraszam za tak "lamerskie" pytanie ale szczerze to nie wiem co mam zrobić.

ktosCZYLIJa napisał(a):

Specyfikację tzn czy mam to opisać jako dokumentację?
Przepraszam za tak "lamerskie" pytanie ale szczerze to nie wiem co mam zrobić.

"wykonywalną specyfikację (również znaną jako testy )". Chodzi o testy automatyczne.

Każdy programista, kiedy tworzy nową rzecz, sprawdza czy jego aplikacja działa, najczęściej ją uruchamiając i testując manualnie nową funkcję. Jednak na dłuższą metę (moim zdaniem projekt na dłużej niż 1h) bardziej się opłaca testować automatycznie, czyli pisząc takie małe snippety kodu które wywołują jakąś funkcję z zadanym wejściem i sprawdzają czy na wyjściu jest spodziewana wartość. Dzięki temu można drastycznie zmniejszyć ilość czasu spędzoną na testach manualnych. Dodatkowo, aplikacja powstała w ten sposób z reguły ma dużo lepszy ogólny design, dlatego że wszelkie błędy projektowe dzięki testom zauważamy dużo wcześniej i możemy je poprawić.

Witam
Mam coś takiego (na początek - czy dobry to się okaże)
Plik Uzytkownk.php:

<?php
namespace Aplikacja;
/**
 * tu jest klasa reprezentująca uzytkownika
 */
class Uzytkownik
{
	//w tej zmiennej będzie przechowywana godność uzytkownika (jego imie i nazwisko)
	private string $godnoscUzytkownika;
	//po utworzeniu obiektu klasy godność uzytkownika jest przypisywana do prywatnej zmiennej $godnoscUzytkownika przyjętej w parametrze konstruktora
	public function __construct($godnoscUzytkownika)
	{
		$this->godnoscUzytkownika = $godnoscUzytkownika;
	}
	//ta funkcja zwraca godność uzytkownika
	public function getGodnoscUzytkownika(): string
	{
		return $this->godnoscUzytkownika;
	}
}

Plik UzytkownikTest.php

<?php
namespace Aplikacja;

//dołaczamy plik Uzytkownik.php z klasą Uzytkownik
require_once("Uzytkownik.php");

use PHPUnit\Framework\TestCase;
use Aplikacja\Uzytkownik;
/*
* tu jest klasa testująca funkcjonalność klasy Uzytkownik 
*/
class UzytkownikTest extends TestCase
{
	//ta funkcja sprawdza czy godność uzytkownika jest typu string
	public function testGetGodnoscUzytkownika()
	{
		//tworzymy nowy obiekt klasy Uzytkownik i przekazujemy mu imie i nazwisko uzytkownika
		$uzytkownik = new Uzytkownik("Jan kowalski");
		//pobieramy imie i nazwisko uzytkownika ustaloną podczas tworzenia obiektu klasy Uzytkownik
		$godnoscUzytkownika = $uzytkownik->getGodnoscUzytkownika();
		//sprawdzamy czy godność uzytkownika jest typu string
		$this->assertIsString($godnoscUzytkownika);
	}
}

Proszę o wyrozumialość, ponieważ z PHPUnit mam do czynienia od wczoraj.
Liczę na konstruktywna krytykę.
Pozdrawiam

ktosCZYLIJa napisał(a):

Witam
Mam coś takiego (na początek - czy dobry to się okaże)
Plik Uzytkownk.php:
[...]
Proszę o wyrozumialość, ponieważ z PHPUnit mam do czynienia od wczoraj.
Liczę na konstruktywna krytykę.
Pozdrawiam

Super! Fajnie że Ci się udało.

Co do kodu - niby w porządku, ale w tym przykładzie który podałeś nie ma logiki. Tzn. aplikacja jakby... nic nie robi. Jak włączysz taki program, to on w zasadzie nic nie zrobi, użytkownik nie ma tego jak użyć. Więc można ostrożnie założyć że ta klasa i ten test na ten moment nie mają sensu. Zastanów się co użytkownik chce zrobić z aplikacją?

Szczerze to ta funkcja miala nie robić nic, napisalem ją w ramach nauki PHPUnit, ale zmienilem na coś takiego:
Plik Uzytkownik.php:

<?php
namespace Aplikacja;
//dołączamy plik z klasą odpowiedzialną za połączenie z bazą danych
require_once("BazaDanych.php");
/**
 * tu jest klasa reprezentująca uzytkownika
 */
class Uzytkownik
{
	//w tej zmiennej będzie przechowywana godność uzytkownika (jego imie i nazwisko)
	public string $godnoscUzytkownika;
	//po utworzeniu obiektu klasy godność uzytkownika jest przypisywana do prywatnej zmiennej $godnoscUzytkownika przyjęta w parametrze konstruktora
	public function __construct(string $godnoscUzytkownika)
	{
		$this->godnoscUzytkownika = $godnoscUzytkownika;
	}
	//ta funkcja zwraca godność uzytkownika
	public function getGodnoscUzytkownika(): string
	{
		return $this->godnoscUzytkownika;
	}
	//funkcja zapisująca uzytkownika w bazie danych
	public function zapiszUzytkownikaWBazie(string $godnoscUzytkownika, BazaDanych $BazaDanych) : bool
	{
		//dodajemy uzytkownika do bazy danych
		$rezultatDodaniaUzytkownikaDoBazyDanych = $BazaDanych->polaczenieZBaza->query("INSERT INTO uzytkownicy () VALUES (null,'" . $godnoscUzytkownika . "')");
		return $rezultatDodaniaUzytkownikaDoBazyDanych;
	}
}

Plik UzytkownikTest.php

<?php
namespace Aplikacja;

//dołaczamy plik Uzytkownik.php z klasą Uzytkownik
require_once("Uzytkownik.php");
//dołączamy plik z klasą odpowiedzialną za połączenie z bazą danych
require_once("BazaDanych.php");

use PHPUnit\Framework\TestCase;
use Aplikacja\Uzytkownik;
use Aplikacja\BazaDanych;
/*
* tu jest klasa testująca funkcjonalność klasy Uzytkownik 
*/
class UzytkownikTest extends TestCase
{
	//ta funkcja sprawdza czy godność uzytkownika jest typu string
	public function testGetGodnoscUzytkownika()
	{
		//tworzymy nowy obiekt klasy Uzytkownik i przekazujemy mu imie i nazwisko uzytkownika
		$uzytkownik = new Uzytkownik("Janek kowalski");
		//pobieramy imie i nazwisko uzytkownika ustalona podczas tworzenia obiektu klasy Uzytkownik
		$godnoscUzytkownika = $uzytkownik->getGodnoscUzytkownika();
		//sprawdzamy czy godność uzytkownika jest typu string
		$this->assertIsString($godnoscUzytkownika);
	}
	//ta funkcja sprawdza czy uzytkownik zapisał sie w bazie danych
	public function testZapiszUzytkownikaWBazie()
	{
		//tworzę nowy obiekt klasy uzytkownik i nadaje mu imie i nazwisko jako jedną wartość string
		$uzytkownik = new Uzytkownik("Janek kowalski");
		//wywoluje funkcje zapisującą uzytkownika w bazie
		$rezultatProbyZapisuUzytkownikaWBazie = $uzytkownik->zapiszUzytkownikaWBazie($uzytkownik->godnoscUzytkownika,new BazaDanych());
		//sprawdzamy czy rezultat wykonania funkcji zapisującej uzytkownika w bazie danych jest wartością logiczną bool
		$this->assertIsBool($rezultatProbyZapisuUzytkownikaWBazie);
	}
}

Tutaj nie wiem czy funkcja zapiszUzytkownikaWBazie nie powinna być raczej klasą z implementacją zapisu uzytkownika w bazie. Mam taką wątpliwość ponieważ kiedyś wyczytalem że każda klasa powinna mieć jedną funkcjonalność, nie wiem jak to rozumieć ?

ktosCZYLIJa napisał(a):

Szczerze to ta funkcja miala nie robić nic, napisalem ją w ramach nauki PHPUnit, ale zmienilem na coś takiego:

Dobra, to biorę na siebie bo źle opisałem problem

ktosCZYLIJa napisał(a):

Tutaj nie wiem czy funkcja zapiszUzytkownikaWBazie nie powinna być raczej klasą z implementacją zapisu uzytkownika w bazie. Mam taką wątpliwość ponieważ kiedyś wyczytalem że każda klasa powinna mieć jedną funkcjonalność, nie wiem jak to rozumieć ?

Nadal jest ten sam problem, że program nic nie robi nie może użyć tego programu do niczego. Jesteś użytkownikiem - chcesz coś zrobić, wykonać jakąś pracę, rozwiązać jakiś problem. Potrzebujesz coś osiągnąć. Przestań na moment myśleć o bazach danych i zastanów się jaki problem możesz rozwiązać dla użytkownika?

Na razie robisz sztukę dla sztuki.

Mam coś takiego - funkcja sprawdza czy podane imie i nazwisko zaiwera spacje lub dwie spacje co w rezultacie daje informację czy podano imię, dwa imiona i nazwisko
Funkcja sprawdzenieCzyPodanoImieINazwisko w pliku Uzytkownik.php

//funkcja sprawdzająca czy podano dwa lub trzy ciagi znaków 
	//zalozeniu ma to być imię i nazwisko lub dwa imiona i nazwisko
	public function sprawdzenieCzyPodanoImieINazwisko(string $godnoscUzytkownika) : bool
	{
		//sprawdzamy czy w dancyh podanych przez uzytkownika jest jedna lub dwie spacje pomiedzy ciągami znaków wprowadzonych przez osobę która potencjalnie nazywa się tak jak podała
		if(substr_count($godnoscUzytkownika," ",1) || substr_count($godnoscUzytkownika," ",2))
			return true;
		else
			return false;
	}

Funkcja w pliku Uzytkownik.php

//ta funkcja sprawdza czy uzytkownik podal prawidlowo imie i nazwisko lub  dwa imienia i nazwisko
	public function testSprawdzenieCzyPodanoImieINazwisko()
	{
		//tworzę nowy obiekt klasy uzytkownik i nadaje mu imie i nazwisko jako jedną wartość string
		$uzytkownik = new Uzytkownik("Janek kowalski");
		//wywoluje funkcje sprawdzająca czy podana wartość jest imieniem lub dwoma imieniami i nazwiskiem
		$rezultaSprawdzenieCzyPodanoImieINazwisko = $uzytkownik->sprawdzenieCzyPodanoImieINazwisko($uzytkownik->godnoscUzytkownika);
		//sprawdzamy czy rezultat wykonania funkcji sprawdzającej czy podano poprawną nazwę użytkownika jest typu bool
		$this->assertIsBool($rezultaSprawdzenieCzyPodanoImieINazwisko);
	}

Pytanie: czy to o coś takiego chodzilo ??
Dziękuję za zaangażowanie w dyskusje.
To dla mnie bardzo ważne.
Pozdrawiam

ktosCZYLIJa napisał(a):

Mam coś takiego - funkcja sprawdza czy podane imie i nazwisko zaiwera spacje lub dwie spacje co w rezultacie daje informację czy podano imię, dwa imiona i nazwisko
Funkcja sprawdzenieCzyPodanoImieINazwisko w pliku Uzytkownik.php
[...]
Pytanie: czy to o coś takiego chodzilo ??
Dziękuję za zaangażowanie w dyskusje.
To dla mnie bardzo ważne.
Pozdrawiam

No powiedzmy że w miare o to chodzi, ale ten test nic nie sprawdza. Funkcja o kodzie {return true;} sprawi że test przechodzi.

Jeśli chciałbyś sensowny test, to bardziej tak:

class UsernameTest extends TestCase {
  public function userHasFirstnameAndLastname() {
    $user = new User('Jan Kowalski');
    $this->assertTrue($user->hasFirstNameAndLastName());
  }

  public function userHasTwoNamesAndLastname() {
    $user = new User('Jan Marek Kowalski');
    $this->assertTrue($user->hasFirstNameAndLastName());
  }

  public function userHasFirstnameAndLastname() {
    $user = new User('Jan');
    $this->assertFalse($user->hasFirstNameAndLastName());
  }

  public function manyWords() {
    $user = new User('aaa bbb ccc ddd');
    $this->assertFalse($user->hasFirstNameAndLastName());
  }
}

Rozumiem, Dziękuję
Muszę poćwiczyć te testowanie ale przyznam że zaczyna mi to się podobać (mieć ręce i nogi).