Czy kod php jest poprawny ?

Cześć, czy mógłby ktoś ocenić czy mój kod php jest, poprawny ? Oczywiście poniższy kod działa tak jak tego oczekuje, ale po prostu chce się dowiedzieć czy poprawnie programuje, czy coś mógłbym lepiej zrobić, inaczej zmienne ponazywać itp. Z góry dzięki.

klasa odpowiedzialna za walidację awatara

class AvatarValidation
{
    private $data;
    private $database;
    private $error = '';


    public function __construct($database, $data)
    {
        $this->database = $database;
        $this->data = $data;
    }

    public function getError()
    {
        return $this->error;
    }

    public function ValidateAvatar()
    {


        $array_extension = array("jpg", "png", "jpeg");
        $val = htmlspecialchars($this->data['avatar']);
        $ext = pathinfo($val, PATHINFO_EXTENSION);

        if(empty($val))
        {
            return $this->error = "Wybierz zdjęcie";
        }
        else if(! in_array($ext, $array_extension) && !(empty($val)))
        {
            return $this->error = "Nieprawidłowe rozszerzenie pliku";
        }

    }
}

?>

klasa odpowiedzialna za dodawanie zdjęcia

class PhotoAdding
{
    private $database;
    private $data;
    private $PhotoValidation;

    public function  __construct($database, $data,  $PhotoValidation)
    {
        $this->database = $database;
        $this->data = $data;
        $this->PhotoValidation = $PhotoValidation;
    }

    public function AddPhoto()
    {
        if(empty($this->PhotoValidation->getError()))
        {
            $id = htmlspecialchars($_SESSION['user_id']);
            $photo = htmlspecialchars($this->data['photo']);
            $date = date('Y-m-d');


            $query = $this->database->ConnectDatabase()->prepare("INSERT INTO photo (`user_id`, `photo`, `date_to_add`) VALUES ((SELECT user_id from user where user_id = :id), :photo, :date)");
            $query->bindParam(':id', $id, PDO::PARAM_INT);
            $query->bindParam(':photo', $photo, PDO::PARAM_STR);
            $query->bindParam(':date', $date, PDO::PARAM_STR);
            $query->execute();
        }
    }
}

Programujesz stanowo w AvatarValidation. Widać to w tym że przypisujesz sobie do aktualnego obiektu jakiś error i dajesz mu $data który powinien być parameterem w funkcji AddPhoto.

Dla przykładu metodę ValidateAvatar zmieniłbym tak, że zwraca jakąś inną klasę Error zamiast zapisywać sobie wewnętrznie errora i pobierać go przez getError()
Druga rzecz to niby zwracasz string jako error, ale potem reagowanie na taki błąd musiałoby polegać na porównywaniu stringów zamiast matchowaniu poprzez instanceof.

PhotoAdding -> tutaj dokładnie to samo. W porządku jest to że odwracasz zależności w konstruktorze ale jednocześnie to $data nie pasuje. Obiekt przestaje być re-używalny bo musisz stworzyć kolejny żeby dodać nowe foto. Gdybyś tylko dał $data do AddPhoto() byłoby prościej.

PS. if(empty($this->PhotoValidation->getError())) jak to działa? Skąd PhotoValidation ma info o photo?

EDIT.
Okej, chyba już rozumiem. $data jest w PhotoAdding i AvatarValidation, pewnie to są te same zmienne.

Tak na szybko:

1. Brak typowania
2. Użyj short array syntax
3. Różny sposób nazywania zmiennych/metod, powinno być cośTam
4. Nie wrzucaj masy kodu to środka ifa, niepotrzebny dodatkowy nesting level. Spokojnie może obrócić warunek, a resztę kodu dać po zamknięciu ifa, np.:

if(!empty($this->PhotoValidation->getError())) {
   return;
}
...

1. Nie sprawdzasz, czy rzeczywiście dodałeś zdjęcie, przydałoby się zwracanie boola - true jeśli dodałeś, a jeśli nie to false. Może też jakiś try/catch warto by dodać.
2. return $this->error = "Wybierz zdjęcie"; - to jakiś potworek. Skoro masz this i potem getError(), to może lepiej zwracać tylko bool?
3. Zamiast jednolinijkowego if-else możesz użyć ? :

Kod do śmieci, klasa class PhotoAdding łączy sie z baza dancyh xD

Nazwy funkcji masz z wielkiej litery np.$PhotoValidation. Treści komunikatów można np. wynieść do stałych np.

Class X
{
    private const NO_PHOTO_ERROR =  "Wybierz zdjęcie"
    function check()
    {
       return $this->error = self::NO_PHOTO_ERROR;
    }
}

Masz wtedy wszystkie komunikaty w jednym miejscu i łatwiej czytać kod i edytować treści.
Tu w SQL wyszukujesz Id po otrzynym Id. Wystarczy chyba Insert

INSERT INTO photo (`user_id`, `photo`, `date_to_add`) VALUES ((SELECT user_id from...)

Reszta już wspomniana.

@jurek1980: Ale czy z takim kodem mam szanse dostac prace, ogólnie wszystko działa, tylko pewnie brzydko jest napisany.

To zależy kogo szukają. Nas pewno nie jesteś samodzielnym liderem, który dostaje temat i go kompleksowo ogarnie. Ale jeśli będą szukali kogoś na staż, juniora albo ludzika do podszkolenia do prostych zadań to myślę, że masz szansę. Ale od razu mówię - nie nastawiaj się na 15k miesięcznie ;)

veks21 napisał(a):

@jurek1980: Ale czy z takim kodem mam szanse dostac prace, ogólnie wszystko działa, tylko pewnie brzydko jest napisany.

Szansa jest zawsze. Zrób to nie jako gołe klasy tylko jako działający system. Poczytaj o PSR to pewnie wyjaśni się camelCase i parę wskazanych tu rzeczy. Próbuj i się nie poddawaj.

Szczerze, ja bym miał opory, żeby przyjąć Cię na juniora, prędzej staż. Z drugiej strony, widziałem już gorszy kod juniorów, więc może nie jest tak źle xD Ale po paru linijkach kodu to co najwyżej mogę zgadywać co potrafisz.

W każdym razie mamy 2021 rok, a używasz składni/formatowania jak z czasów PHP 5, a teraz mamy już PHP 8^^ Pierwsze co powinieneś zrobić, to zapoznać się z nowościami. Plus poczytać sobie o linterach do PHP.

@serek: Czyli główny problem leży w formatowaniu kodu, ze nieuzywam wersji 8 php, tak ?

veks21 napisał(a):

@serek: Czyli główny problem leży w formatowaniu kodu, ze nieuzywam wersji 8 php, tak ?

Nie, ale dużo lepiej to będzie to wyglądać.

@jurek1980:
Zmieniłem troche kod, aby funkcja zwracała określony typ danych.

class AvatarValidation
{
    private $data;
    private $database;
    private string $error = '';

    public function __construct($database, $data)
    {
        $this->database = $database;
        $this->data = $data;
    }

    public function getError(): string
    {
        return $this->error;
    }

    public function ValidateAvatar(): string
    {

        $array_extension = array("jpg", "png", "jpeg");
        $val = htmlspecialchars($this->data['avatar']);
        $ext = pathinfo($val, PATHINFO_EXTENSION);

        if(empty($val))
        {
            return $this->error = "Wybierz zdjęcie";
        }
        else if(! in_array($ext, $array_extension) && !(empty($val)))
        {
            return $this->error = "Nieprawidłowe rozszerzenie pliku";
        }

    }
}

To teraz na początku pliku daj
declare(strict_types=1); i zobacz co się zaświeci i spróbuj tak kod napisać by IDE nie okazywało błędów a progrma ciągle działa, tylko rób to jako wersja 1.1

Dodatkowo jeszcze:

1. elseif zwykle się pisze to razem
2. po co && !(empty($val), zbędna redundancja
3. po negacji nie dawaj spacji, lepiej wygląda !in_array

@jurek1980: Teraz w ogóle nie działa

Jak dla mnie to trochę mało tego kodu żeby coś więcej powiedzieć, no bo co my tu mamy:

• dwie klasy (jedna do walidacji druga do zapisu do bazy)
• klasa walidująca posiada konstruktor (gdzie przypisywane jest $database, które jest używane... no właśnie... gdzie?)
• nazwy metod, zmiennych wcale nie przypominają tego co mają robić (metoda getError zwraca error natomiast metoda ValidateAvatar zwraca stringa, który notabene jest tym samym co getError albo nie zwraca NIC :) )
• o nazewnictwie zmiennych to już pisano wcześniej

Generalnie z tego co pokazałeś to wiesz jak:

• zrobić klasę
• napisać do niej kilka metod
• i... (dopiszcie jak komuś coś jeszcze wpadnie)

Samo stworzenie klasy to jeszcze nie będzie OOP :)

Odpowiadając na Twoje najważniejsze pytanie "czy dostanę jakąś pracę" - z takim kodem raczej nie masz szans na pracę w żadnej sensownej firmie. Jeśli faktycznie umiesz napisać coś większego co działa i ogarniasz trochę front to zapewne znajdziesz pracę w jakiejś firmie wdrażającej proste CMS'y - tam jest potrzeba na ludzi, którzy ogólnie ogarniają, a jakość kodu jest na ostatnim miejscu.

Jeśli faktycznie umiesz pisać większe aplikacje, które działają (z tego kodu niestety nikt Ci nie oceni), znasz jakiś framework, to jesteś jednak w całkiem dobrej sytuacji. Moim zdaniem o wiele trudniej jest dojść do poziomu "zero" do "ogarniam" niż od poziomu "ogarniam" do "umiem pisać ładny kod". To jest po prostu wtedy tylko kwestia kogoś kto pokaże Ci odpowiednie wzorce i Twojej woli aby je stosować.

Co bym zrobił na Twoim miejscu to po pierwsze zadbał o kod na poziomie statycznym - to co zaprezentowałeś tutaj to jest automatyczna dyskwalifikacja w większości firm ze względu na to co koledzy pisali wyżej - nikt nawet nie będzie analizował Twojej aplikacji dalej. Trochę jakbyś przyszedł ubrany jak bezdomny do banku i poprosił o usługę private banking - może i masz milion dolarów, ale wyglądasz jak bezdomny więc Cię ochrona wyprosi zanim dotrzesz do odpowiedniej osoby.

Dlatego:

• tak jak pisał @jurek1980 używaj strict types + typowania wszystkich pól klas, parametrów metod i zwracanych typów - kod Ci przestał działać? To bardzo dobrze - znaczy, ze wychwyciłeś błąd, który mógłby przysporzyć sporo kłopotów na produkcji
• zainteresuj się PHP8 - @serek tutaj trafnie zauważył, że Twój kod wygląda jakbyś się uczył programowania z książek do PHP5 - miałem identyczne skojrzaenie
• zainteresuj się tym: https://phptherightway.com/#code_style_guide - to są szalenie ważne rzeczy w komercyjnym kodzie i dzięki temu Twój kod przestanie wyglądać jak bezdomny (przynajmniej na poziomie edycyjnym)
• jak to ogarniesz to polecam książkę "Czysty kod" - całej zapewne nie zrozumiesz, ale pierwsze kilka rozdziałów to jest dokładnie to czego potrzebujesz

Dalej jak już poprawisz swój kod od strony wizualnej to pora poprawić jego organizację. Tutaj niestety są to lata pracy i ciągle będziesz widział coś do poprawy. Zacząłbym od:

• nauczenia się phpUnit i rozpoczęcia pisania testów jednostkowych
• przeczytania jakiejś książki lub obejrzenia jakiegoś dobrego tutoriala o MVC - coś bez frameworka, żeby załapać jak to działa od środka
• potem próbowałbym masterować jakiś framework typu Symfony + czytał sporo o kodzie, architekturze itp.
• po 5-10 latach poczujesz się dość komfortowo ;p

veks21 napisał(a):

Tak, jak napisałem, stworzyłem dużą aplikację bardzo podobną do facebooka, która działa, wiec umiem coś większego napisać, dużo gorzej jest tylko z czytelnością kodu, będe próbwać jakoiś poprawić ten kod, ale juz widze ze nie będze to łatwe zmieniać, szkoda ze nie stosowałem takich praktyk jak pisałem aplikacje

Najlepiej pokaż kod tej aplikacji, to Ci powiemy jak możesz poprawić jej czytelność.

PS: Masz testy do niej?

@TomRiddle:
Poprawiłem trochę te dwie klasy, mam tylko problem jaki typ ma zwracać funkcja validateAvatar, bo ona zwraca zarówno stringa jak i void, a nie mogę dać uni tych dwóch typów, wiec nie wiem, pewnie bede musiał jakoś poważniej tą walidacje zmodyfikować.

<?php

declare(strict_types=1);

class PhotoAdding
{

    public function  __construct(
        private $database,
        private  array $data,
        private  $PhotoValidation,
    ){}

    public function AddPhoto(): void
    {
        $id = htmlspecialchars($_SESSION['user_id']);
        $photo = htmlspecialchars($this->data['photo']);
        $date = date('Y-m-d');

        if(empty($this->PhotoValidation->getError()))
        {
            $query = $this->database->ConnectDatabase()->prepare("INSERT INTO photo (`user_id`, `photo`, `date_to_add`) VALUES ((SELECT user_id from user where user_id = :id), :photo, :date)");
            $query->bindParam(':id', $id, PDO::PARAM_INT);
            $query->bindParam(':photo', $photo, PDO::PARAM_STR);
            $query->bindParam(':date', $date, PDO::PARAM_STR);
            $query->execute();
        }
    }
}
?>

<?php

declare(strict_types=1);

class AvatarValidation
{
    private string $error = '';

    public function __construct(
        private  $database,
        private array $data,
    ){}

    public function getError(): string
    {
        return $this->error;
    }

    public function ValidateAvatar()
    {
        $array_extension = array("jpg", "png", "jpeg");
        $val = htmlspecialchars($this->data['avatar']);
        $ext = pathinfo($val, PATHINFO_EXTENSION);

        if(empty($val))
        {
            return $this->error = "Wybierz zdjęcie";
        }
        elseif(!in_array($ext, $array_extension) && !(empty($val)))
        {
            return $this->error = "Nieprawidłowe rozszerzenie pliku";
        }

    }
}

?>

Ciągle masz nazwy funkcji z wielkiej litery.
Odnośnie pytania, jeśli błędy wpisuejez w pole klasy, to nie ma sensu chyba zwracać też ich wartości? Mi zawsze walidacja kojarzy się z bool i true dla ok, false dla błędu.
Natomiast wyświetlenie błędów możesz zrobić już po walidacji, jeśli ona nie przejdzie prawidłowo.
BTW metoda jest publiczna. Przydałoby się ją przetestować. W przypadku bool łatwo napiszesz unit testy.

@jurek1980:

Czy o taką walidację Ci chodziło ?

class AvatarValidation
{

    private string $error = '';

    public function __construct(
        private  $database,
        private array $data,
    ){}

    public function getError(): string
    {
        return $this->error;
    }

    public function ValidateAvatar(): bool
    {
        $array_extension = array("jpg", "png", "jpeg");
        $val = htmlspecialchars($this->data['avatar']);
        $ext = pathinfo($val, PATHINFO_EXTENSION);

        if(empty($val))
        {
            return false;
        }
        elseif(!in_array($ext, $array_extension) && !(empty($val)))
        {
            return false;
        }else
            return true;;

    }
}

Bliżej. Tylko pozbyleś się tych informacji o błędach. Może dać je do jakichś podmetod? Zastanów się.
Popatrz też na drugi warunek. Sprawdzasz czy val jest empty, a zrobiłeś to już wcześniej i dałeś return.

veks21 napisał(a):

Poprawiłem trochę te dwie klasy, mam tylko problem jaki typ ma zwracać funkcja validateAvatar(), bo ona zwraca zarówno stringa jak i void, a nie mogę dać unii tych dwóch typów, wiec nie wiem, pewnie bede musiał jakoś poważniej tą walidacje zmodyfikować.

Tragedia.

• Po pierwsze, to funkcja nie zwraca nigdy void, co najwyżej zwraca null.
• Po drugie, pomyślałeś może kiedyś czemu nie ma unii typów? Dlatego że taki design jest bardzo słaby. Idealnie funkcja powinna albo zawsze zwracać string albo zawsze nic.
• Po trzecie, NIGDY ale to nigdy, nie łącz HTMLa z SQL. To że robisz htmlspecialchars() zanim wsadzisz do bazy danych to w najlepszym wypadku recepta na bugi, w najgorszym niesamowite narażenie się na luki w bezpieczeństwie. Nigdy nie rób htmlspecialchars() przed wkładaniem danych do bazy; zrób to dopiero przy wyświetlaniu tych danych, i to tylko jeśli pokazujesz je w HTML.

Co do kodu samego, to pytanie czy używasz tego getError() jeszcze w innym miejscu? Czy tylko w tym kodzie którym pokazałeś? Jeśli używasz go gdzieś indziej, to wklej również ten kod.

@TomRiddle:

Idealnie funkcja powinna albo zawsze zwracać string albo zawsze nic.

Zbyt duże uogólnienie. Ktoś tak jeszcze pomyśli. Poza tym rozszerz swój pogląd, dlaczego funkcja walidujaca powinna według Ciebie zwracać Void?

Chłopaki tutaj mówią o architekturze więc ja trochę pochylę się nad samym kodem. Ta klasa ma sporo rzeczy do poprawy. To co musisz zrozumieć, to w kodzie komercyjnym bardzo ważna jest czytelność. Tymczasem Twoja klasa jest bardzo zawiła i nieczytelna + zawiera sporo martwego kodu. Kilka moich spostrzeżeń:

• nazwa wydaje mi się dziwna - raczej powinno być AvatarValidator
• nie rozumiem poco tutaj konstruktor / stan w tej klasie. Database nigdzie nie jest używany (i dobrze), a przekazywanie danych w konstruktorze też trochę nie ma sensu. Lepiej przekazać path od razu do funkcji validatora.
• po co funkcja getError()? Przecież nigdzie w obecnej klasie nie masz tych błędów, więc zawsze będzie zwracany pusty string
• dlaczego nazwa funkcji z dużej litery? W php konwencja dla nazw funkcji i zmiennych to camelCase
• tak jak pisałem, wg mnie znacznie ładniej byłoby ścieżkę do pliku przekazać do funkcji, a nie przekazywać tam całą tablicę $data - robisz niepotrzebne zależności. Skoro funkcja potrzebuję tylko ścieżki do pliku to tylko ścieżkę powinna otrzymać. Dzięki temu funkcja jest łatwiejsza do zrozumienia i przetestowania
• linia pierwsza - ponownie zmienne w php to camelCase. Druga sprawa to dodawanie typu zmiennej w nazwie jest bez sensu. Powinno być raczej coś w stylu $allowedMediaFileExtensions. Używasz w tej linii również przestarzałej konstrukcji array() - obecnie używamy []. Stosujesz też podwójne cudzysłowy - w statycznych stringach powinno się stosować pojedyńcze bo taka jest konwencja + jest to trochę wydajniejsze. Wreszcie czy ta tablica to zmienna? Nie wydaje mi się - to raczej jest stała, więc lepiej to przenieść do stałej. W przyszłości ułatwi to refacoring i będzie bardziej jasne dla programisty
• linijka kolejna. Musisz zdecydowanie popracować nad nazewnictwem zmiennych. $val, $ext to nie są jednoznaczne zmienne. Nazwa zmiennej powinna przekazywać co w niej jest, bo jak za miesiąc wrócisz to nie będziesz musiał analizować kodu.
• no i jeszcze ten okropny zapis warunków logicznych...

Po pierwsze powinieneś jak ognia unikać konstrukcji if/else bo bardzo ogranicza ona czytelność kodu. Po drugie w Twoim kodzie elsy zastosowałeś zupełnie zbędznie. Po co elseif skoro jeśli warunek w if jest spełniony to funkcja w tym miejscu się kończy bo jest return?
Zastanów się też nad sensem używania konstrukcji w stylu if $val === true return true else return false - bo dokładnie takiej konstrukcji użyłeś. Przecież wystarczy zrobić return $val

Jeśli dobrze rozumiem koncept Twojej klasy to można by ją zapisać mniej więcej tak:

<?php

declare(strict_types=1);

class AvatarValidator
{
    private const ALLOWED_MEDIA_FILE_EXTENSIONS = ['jpg', 'png', 'jpeg'];

    public function validate(string $path): bool
    {
        if (!$path) {
            return false;
        }

        return in_array(
            pathinfo($path, PATHINFO_EXTENSION),
            self::ALLOWED_MEDIA_FILE_EXTENSIONS
        );
    }
}

jurek1980 napisał(a):

@TomRiddle:

Idealnie funkcja powinna albo zawsze zwracać string albo zawsze nic.

Zbyt duże uogólnienie. Ktoś tak jeszcze pomyśli. Poza tym rozszerz swój pogląd, dlaczego funkcja walidujaca powinna według Ciebie zwracać Void?

Ale ja nic takiego nie mówiłem.

Mówiłem że funkcje w ogóle które raz zwracają string a raz null to słaby design.

veks21 napisał(a):

Poprawiłem trochę te dwie klasy, mam tylko problem jaki typ ma zwracać funkcja validateAvatar, bo ona zwraca zarówno stringa jak i void, a nie mogę dać uni tych dwóch typów, wiec nie wiem, pewnie bede musiał jakoś poważniej tą walidacje zmodyfikować.

A po czwate w sumie, kolejny błąd, u Ciebie operujesz na tej zmiennej $this->data. To jest zbyt zserializowana dana, powinieneś ją odpakować dużo wcześniej, jak ją tylko dostaniesz. Ja bym widział tak ten kod:

<?php
class PhotoRepository
{
    public function __construct(private $database)
    {
    }

    public function AddPhoto(int $id, string $photo, string $date): void
    {
        $query = $this->database->ConnectDatabase()->prepare("INSERT INTO photo (`user_id`, `photo`, `date_to_add`) VALUES ((SELECT user_id from user where user_id = :id), :photo, :date)");
        $query->bindParam(':id', $id, PDO::PARAM_INT);
        $query->bindParam(':photo', $photo, PDO::PARAM_STR);
        $query->bindParam(':date', $date, PDO::PARAM_STR);
        $query->execute();
    }
}

class Avatar
{
    public function __construct(private string $avatar)
    {
    }

    public function getAvatar(): string
    {
        if ($this->avatar === '') {
            throw new ValidationException("Wybierz zdjęcie");
        }
        if (!in_array($this->getExtension(), ['jpg', 'png', 'jpeg'])) {
            throw new ValidationException("Nieprawidłowe rozszerzenie pliku");
        }
        return $this->avatar;
    }

    private function getExtension(): string
    {
        return pathinfo($this->avatar, PATHINFO_EXTENSION);
    }
}

class ValidationException extends Exception {}

Użycie

$avatar = new Avatar($this->data['photo']);
$repository = new PhotoRepository($database);
try {
    $repository->AddPhoto($_SESSION['user_id'], $avatar->getAvatar(), date('Y-m-d'));
}
catch (ValidationException $exception) {
    $errorMessage = $exception->getMessage();
    // pokaż wiadomość userowi
}

@TomRiddle: Tylko ten kod jest niepoprawny, ponieważ tworząc obiekt klasy Avatar powinno być użyte $_POST, a nie $this->data['photo'].

@TomRiddle:
Taki kod działa,poprawnie, trochę jest inny od twojego ale twój pkazywał kilka błedów

<?php

declare(strict_types=1);

class Avatar
{
    public function __construct(
        private $data,
    ){}
    
    public function getAvatar(): string
    {
        $val = $this->data['avatar'];

        if(empty($val))
        {
            throw new ValidationException("Wybierz zdjęcie");
        }
        else if(!in_array($this->getExtension(), ['jpg', 'png', 'jpeg']))
        {
            throw new ValidationException("Nieprawidłowe rozszerzenie pliku");
        }
        return $val;

    }

    private function getExtension(): string
    {
        $val = $this->data['avatar'];

        return pathinfo($val, PATHINFO_EXTENSION);
    }
}

class ValidationException extends Exception {}
?>

<?php

declare(strict_types=1);

class AvatarUpdating
{

    public function  __construct(
        private $database,
    ){}

    public function UpdateAvatar(int $id, string $photo): void
    {
            $query = $this->database->ConnectDatabase()->prepare("UPDATE user set avatar = :avatar where user_id = :id");
            $query->bindParam(':avatar', $photo, PDO::PARAM_STR);
            $query->bindParam(':id', $id, PDO::PARAM_INT);
            $query->execute();
    }
}

?>

$avatarValidation = new Avatar($_POST);
$update_avatar = new AvatarUpdating($database);

<?php
                        if(isset($_POST['submit_avatar']))
                        {
                            try {
                                    $update_avatar->UpdateAvatar($_SESSION['user_id'], $avatarValidation->getAvatar());
                                }catch (ValidationException $exception)
                                {
                                    echo $errorMessage = $exception->getMessage();
                                }
                        }
                        ?>