Trebam reviziju PHP klase i principa OOP-a :)

dmitrecic · September 17, 2018, 12:21pm

@tpojka prvo da ti se zahvalim za link na PHP klase i objekte i na standarde. Pomaže vrlo
Zamolio bih tebe (i one koji imaju iskustva s OOP) da mi nakratko proletiš kroz klasu koju sam danas napravio (prateći principe i standarde koliko sam uspio ) i kažeš svoje mišljenje o:

da li je ispravno napisan? (OOP standardi, principi…)
radi li se to na taj način? (da li bi ti napravio tako?)
prolazi li ovakav code ili postoje greške koje sam napravio u njemu (sam code je ispravan i funkcionalan)
na što moram pripaziti kod pisanja ovakvih klasa (primjerice, jesam li ga mogao kako skratiti koristeći principe oop-a)?

FtpUploadToServer.class.php:

<?php

class FtpUploadToServer
{
const VERSION="1.0";

public $ftp_folder="";
public $ftp_file="";
public $createfolder=0;
public $fileoverwrite=1;
public $showmessages=1;
public $timeout=60;
public $passivemode=1;
public $uploadmode=FTP_ASCII; // or FTP_BINARY

function __construct($hostname, $username, $password,$port ){
    $this->hostname=$hostname;
    $this->username=$username;
    $this->password=$password;
    $this->port=$port;
}

public function ftpUpload()
{
    $this->showMessage("PHP FTP uploader Version ".self::VERSION);
    if ($this->connectToServer()) {
        $this->uploadFile();
        $this->closeConnection();
    }
}

private function connectToServer()
{
       $this->ftpconn_id=@ftp_connect ($this->hostname , $this->port, $this->timeout);
       if(is_resource($this->ftpconn_id)) {
            if (@ftp_login ($this->ftpconn_id, $this->username, $this->password)){
                @ftp_pasv($this->ftpconn_id,$this->passivemode);
                $this->showMessage("FTP connection established");
                $this->showMessage("Logged in");
                return true;

            } else {
                $this->showMessage("Login error: Please check your username and password for the host ".$this->hostname);

            }
        } else {
            $this->showMessage("Connection error: check hostname and port (current Host:".$this->hostname."  Port:".$this->port.")");
        }
    return false;
}

private function uploadFile()
{
    if(@ftp_chdir($this->ftpconn_id,$this->ftp_folder)) {
        $remotefileexists=@ftp_size($this->ftpconn_id, $this->ftp_file);

        if ($remotefileexists<0 || ($remotefileexists>=0 && $this->fileoverwrite==1 )) {
            if(ftp_put($this->ftpconn_id,$this->ftp_file,$this->ftp_file, $this->uploadmode))
            {
                $this->showMessage("File ".$this->ftp_file." successfuly uploaded");
                return true;

            } else {
                $this->showMessage("Error! File ".$this->ftp_file." cannot be uploaded!");
            }

        } else {
            $this->showMessage("File ".$this->ftp_file." exists! File not uploaded - fileoverwrite disabled!");
        }

    } else {
        if ($this->createfolder==1)
        {
            if (@ftp_mkdir($this->ftpconn_id, $this->ftp_folder))
            {
                $this->uploadFile();
            } else {
                $this->showMessage("Cannot create folder at host ".$this->hostname);
            }

        } else {
            $this->showMessage($this->hostname." directory ".$this->ftp_folder." not exists!");
           

        }
        
    }

    return false;
    
}


private function closeConnection()
{
    ftp_close($this->ftpconn_id);
    $this->showMessage("FTP connection closed");
    return true;
}

private function showMessage($message)
{
    if ($this->showmessages)
    {
        echo $message."<br/>";
    }
}

}

?>

Korištenje:

index.php:

<?php
require("classes/FtpUploadToServer.class.php");
$ftp=new FtpUploadToServer("example.com","korisnickoime","lozinka","21");
$ftp->showmessages=1;
$ftp->ftp_folder="testfolder";
$ftp->ftp_file="test.txt";
$ftp->createfolder=1;
$ftp->fileoverwrite=1;
$ftp->ftpUpload();

?>

Pa eto molim, osim prozvanoga mladog gospona, ako tko drugi ima iskustva - molio bih pregled i komentar (da dobijem smjernice u radu s objektima).
Zauzvrat sam spreman platit pivu!
Hvala!

dmitrecic · September 17, 2018, 12:28pm

htio sam napraviti nešto od čega bih imao i koristi. Dakle ovo bih iskoristio za updatiranje datoteka na 12 različitih domena na kojima stranice koriste iste skripte, pa bi mi bilo dovoljno napraviti izmjenu na jednoj datoteci i tada pustiti ovu skriptu da ftp-om updatira tu datoteku na svim stranicama.

dmitrecic · September 17, 2018, 2:32pm

Dok se netko javi, ja već napravih reviziju

<?php

class FtpUploadToServer
{
    const VERSION="1.1";

    private $ftp_folder="";
    private $ftp_file="";
    private $createfolder=0;
    private $fileoverwrite=1;
    private $showmessages=1;
    private $timeout=60;
    private $passivemode=1;
    private $uploadmode=FTP_ASCII; // or FTP_BINARY

    public function setFolder($folder)
    {
        $this->ftp_folder=$folder;
    }

    public function setFile($file)
    {
        $this->ftp_file=$file;
    }

    public function setCreateFolder($cfolder)
    {
        $this->createfolder=$cfolder;
    }

    public function setFileOverwrite($foverwrite)
    {
        $this->fileoverwrite=$foverwrite;
    }

    public function setShowMessages($smessages)
    {
        $this->showmessages=$smessages;
    }

    public function setTimeOut($stimeout)
    {
        $this->timeout=$stimeout;
    }

    public function setPassiveMode($spmode)
    {
        $this->passivemode=$spmode;
    }

    public function setUploadMode($sumode)
    {
        $this->uploadmode=$sumode;
    }


    function __construct($hostname, $username, $password,$port ){
        $this->hostname=$hostname;
        $this->username=$username;
        $this->password=$password;
        $this->port=$port;
    }

    public function ftpUpload()
    {
        $this->showMessage("PHP FTP uploader Version ".self::VERSION);
        if ($this->connectToServer()) {
            $this->uploadFile();
            $this->closeConnection();
        }
    }

    private function connectToServer()
    {
           $this->ftpconn_id=@ftp_connect ($this->hostname , $this->port, $this->timeout);
           if(is_resource($this->ftpconn_id)) {
                if (@ftp_login ($this->ftpconn_id, $this->username, $this->password)){
                    @ftp_pasv($this->ftpconn_id,$this->passivemode);
                    $this->showMessage("FTP connection established");
                    $this->showMessage("Logged in");
                    return true;

                } else {
                    $this->showMessage("Login error: Please check your username and password for the host ".$this->hostname);

                }
            } else {
                $this->showMessage("Connection error: check hostname and port (current Host:".$this->hostname."  Port:".$this->port.")");
            }
        return false;
    }

    private function uploadFile()
    {
        if(@ftp_chdir($this->ftpconn_id,$this->ftp_folder)) {
            $remotefileexists=@ftp_size($this->ftpconn_id, $this->ftp_file);

            if ($remotefileexists<0 || ($remotefileexists>=0 && $this->fileoverwrite==1 )) {
                if(ftp_put($this->ftpconn_id,$this->ftp_file,$this->ftp_file, $this->uploadmode))
                {
                    $this->showMessage("File ".$this->ftp_file." successfuly uploaded");
                    return true;

                } else {
                    $this->showMessage("Error! File ".$this->ftp_file." cannot be uploaded!");
                }

            } else {
                $this->showMessage("File ".$this->ftp_file." exists! File not uploaded - fileoverwrite disabled!");
            }

        } else {
            if ($this->createfolder==1)
            {
                if (@ftp_mkdir($this->ftpconn_id, $this->ftp_folder))
                {
                    $this->uploadFile();
                } else {
                    $this->showMessage("Cannot create folder at host ".$this->hostname);
                }

            } else {
                $this->showMessage($this->hostname." directory ".$this->ftp_folder." not exists!");
               

            }
            
        }

        return false;
        
    }


    private function closeConnection()
    {
        ftp_close($this->ftpconn_id);
        $this->showMessage("FTP connection closed");
        return true;
    }

    private function showMessage($message)
    {
        if ($this->showmessages)
        {
            echo $message."<br/>";
        }
    }

}

?>

Izmjena i u korištenju klase:

<?php
require("classes/FtpUploadToServer.class.php");
$ftp=new FtpUploadToServer("ftp.example.com","username","password","21");
$ftp->setShowMessages(1);
$ftp->setFolder("testclass");
$ftp->setFile("test.txt");
$ftp->setCreateFolder(1);
$ftp->setFileOverwrite(1);
$ftp->ftpUpload();

?>

jorgovan · September 17, 2018, 6:05pm

@dmitrecic

Da li to pokusavas napraviti skriptu za deploy web stranica/aplikacija?

Prouci kako se radi dev ops.

dmitrecic · September 17, 2018, 6:20pm

Ne, ovo je samo osobno testiranje/izrada klase i “prebacivanje” na OOP način razmišljanja i pisanja code-a. Zanima me jel ispravno u tom smislu.
Što se same funkcionalnosti tiče, imam 17 stranica koje pogoni ista skripta, pa kada napravim izmjenu na jednoj datoteci (recimo, želim ubaciti reklamu ili izmjeniti dio upita na bazu) da se ne moram 17 puta ulogirati ftp klijentom na svaku domenu zasebno i ulaziti u foldere da bih uplodirao tu jednu datoteku. Ovako podignem datoteku na “glavnu” stranicu, pokrenem skriptu i ona obavi upload na ostalim domenama.

ivanm · September 18, 2018, 11:16am

Samo jedan mali prijedlog, neke metode vraćaju false ili ispisuje poruku. Ovo bi trebalo biti konzistentno, znaci uvijek je output isti data type.

Moj prijedlog:

vrati array sa statusom i porukom, npr: array(‘status’ => true, ‘msg’ =>‘bla bla’)
U odvojenoj metodi napravi validaciju i ispiši poruku.

dmitrecic · September 18, 2018, 11:54am

Hvala, to je dio koji mi je promakao: “budi konzistentan”
Promakne ako ne obratiš pozornost, eto meni promaklo.
Ako se tko pita zašto sam ovo stavio na procjenu: ovo je prva klasa koju sam napisao ikada (od početka do kraja) pa me zanimalo da li je ispravno napisana (po principima i standardima) i da li da nastavim s ovakvom praksom ili trebam mijenjati “mindset”.
Ovo je dobar savjet i usvojiti ću ga. Hvala!

ivanm · September 18, 2018, 12:58pm

Generalno izgleda dosta dobro, ima tu sitnica koje se još daju doraditi. Ali za prvu samostalnu klasu je stvarno OK.

Inace jos jedan hint, nije nesto bitno i vise je do code styla ali mozda bude korisno.

Ti npr imas ovo:

   if(is_resource($this->ftpconn_id)) {
                if (@ftp_login ($this->ftpconn_id, $this->username, $this->password)){
                    @ftp_pasv($this->ftpconn_id,$this->passivemode);
                    $this->showMessage("FTP connection established");
                    $this->showMessage("Logged in");
                    return true;

                } else {
                    $this->showMessage("Login error: Please check your username and password for the host ".$this->hostname);

                }
            } else {
                $this->showMessage("Connection error: check hostname and port (current Host:".$this->hostname."  Port:".$this->port.")");
            }

cyclomatic complexity ces spustiti ako smanjiš broj nested conditiona. Znaci isti kod moze sa manje nested conditiona bi izgledao ovako

 if ( ! $remotefileexists<0 || ! ($remotefileexists>=0 && $this->fileoverwrite==1 )) {
	$this->showMessage("File ".$this->ftp_file." exists! File not uploaded - fileoverwrite disabled!");
	return false;
}

if( ! ftp_put($this->ftpconn_id,$this->ftp_file,$this->ftp_file, $this->uploadmode))
{
	$this->showMessage("File ".$this->ftp_file." successfuly uploaded");
	return true;

} else {
	$this->showMessage("Error! File ".$this->ftp_file." cannot be uploaded!");
}

Ovo gore je samo jedan primjer, mozes primjeniti mozda na vise metoda, nisam analizirao

Jos jedna ideja kad već tipkam, napravi class property u koji ces spremati poruke, dodaj nekakav setter za to i pushaj poruke u array. Napravis getter kao public methodu i kasnije samo iste poruke ispišeš negdje.

primjer:

public $messages = array();

private function setMessage($msg){
    $this->messages[] = $msg;
}

public function getMessages(){
    return $this->messages;
}

dmitrecic · September 18, 2018, 1:48pm

Hvala.
Mogao sam dodati još metoda (npr za SFTP), međutim stao sam jer mi je bilo samo bitno da dam na ocjenu vama radim li ispravno i koristim li objekt ispravno.
Sam kod znam da mogu ispeglati (skratiti), međutim nisam htio jer mi je želja bila (sad za prvu) što više istipkati (a da opet ne bude previše) da se “uvedem” u OOP.
Navika iz proceduralnog mora u mirovinu…

tpojka · September 19, 2018, 10:29am

Pratim temu redovno, samo ne nalazim vremena ovih dana da odgovorim.
Prvo, slażem se sa svim što je @ivanm naveo.
Primjera radi, trenutno radim na nečijem kodu koji ima 5-6 nested if petlji. Prosto je nemoguće apstrahovati sve moguće vrijednosti i kombinacije na izlazu. A da stvar bude grdja, nerijetko na izlazu iz if ili elseif petlje stoji neki redirect (jer dalja operacija ne bi trebala biti moguća, je l’?) što bi bilo da te opcije treba postaviti na početku. Upravo ovo kako je pomenuto gore treba uraditi nested if (nekad i nešto drugo pupout switch ili while ali ovim gore se neće pogriješiti).
Upotreba setter-a i getter-a će ti olakšati rad u mnogočemu.
Jedino što bi’ dod’o je da setter treba da vrati instancu a ne void pa na taj način može da se vrši method chaining:

public function setDir($dir)
{
    $this->ftpDir = $dir;
    return $this;
}

Setter-i treba da vraćaju mutirani objekt klase tako da se ne misli šta je u varijabli. Uvijek je objekt iste klase nad kojim je izvršena ta neka posljednja izmjena.
Primjera radi

$ftp = new Ftp();
$ftp->setDir('/path/to/dir')->download($source, $dest, $host);
// $ftp je sad objekt sa postavljenim direktorijumom

Ali sad ću ti reći kako sam pristupio ovoj temi. Kako se sva moja prethodna (i sadašnja) obuka svodi na konkretna rješenja kad zatrebaju, kad sam vidio topic, prvo što sam uradio je da sam zagugl’o “github php ftp upload” i od ponudjenih linkova vidim da je ovaj izražen/korišten (brojem zvjezdica za repo, datumom posljednjeg održavanja…) i vidim da ima solidnu sintaksu. Dakle da bi’ koristio PHP FTP klijent treba da samo da otkucam u terminalu composer require nicolab/php-ftp-client i sav kod mi je u projektu. Ono što mi se svidja je što je i sam autor napomenuo kako da se proširi kod sopstvenim klasama/funkcionalnostima.
Ono što želim da kažem je da je već napisan kod koji mi treba i brže će biti da ga koristim (naravno ne sve i svašta već github repo-e sa više hiljada zvezdica ili bar sa više stotina) nego da pišem iz početka i rizikujem da nešto bitno zaboravim.
Iz istog razloga sam i počeo rad sa PHP FW-cima na samom početku svoje PHP karijere - jer više stotina/hiljada ljudi je već riješilo dječje bolesti koje bi’ treb’o preležavati da sam pametov’o svojim kodom/FW-om. Usput će se lako savladavati šta i kako i zbog čega je bolje. Do tad samo treba da prihvatim uvriježene načine i trenutne standarde.

Btw, prva klasa ti izgleda bolje nego moja prva klasa. Mislim da je bila neka kombinacija OOP-a i špageti funkcija a u vezi rename-ovanja fajlova ili resize-ovanja fotki (ne sjećam se tačno) ali znam da mi je trebalo nekoliko sati da to odradim ručno te sam se upustio u iznalaženje automatskog rješenja.

dmitrecic · September 19, 2018, 10:44am

Hvala na analizi, savjetima i primjerima! Svakako ću primjeniti tu praksu.
Znam i za github i da postoje već slična (bolja) rješenja, međutim morao sam negdje početi (služeći se linkovima na PSR standarde, osnovne principe oop-a sam znao ali ih sada istinski proučavam nasljeđivanje koje mi je također jasno, jedino mi apstraktne klase još nisu legle samo jer ne vidim zašto bih koristio njih, ali saznat ću ).
Kažem, bitno mi je da znam da idem u pravom smjeru, jer ako sad u startu ne krenem ispravno - desit će se problem kakav ti imaš sa raspetljavanjem tuđeg koda (ne bih da netko gleda moj kod i sjeti mi se mame… ).
Ponovno, hvala!

dmitrecic · September 20, 2018, 10:53am

Otvorih si račun na github-u, napravih nove klase (ovaj puta za prijevod teksta koristeći Yandexov Translation API servis).
Jedna je osnovna (YandexTranslate - za spajanje na Yandex i prijevod teksta), druga je ekstenzija prve klase i služi za spremanje prijevoda u bazu (YandexTranslationSave).

Eto, prakticiram OOP, trudim se, kad savladam malo više, idem na Laravel punom parom

tpojka · September 21, 2018, 9:51am

Bravo.
Kad stigneš, obrati pažnju na pravilno pripremljenu strukturu PHP paketa (s posebnim osvrtom na composer.json fajl). U tome se krije tajna plug’n’play PHP paketa.
Uopšteno, ThePhpLeague koristi dobru praksu pa se može pogledati funkcionalnost i struktura paketa koji se nude pod njihovim imenom.
Mislim da može dobro doći i ovaj tekst.

Što se tiče koda, već skoro 3 godine (otkako je PHP7 izaš’o, a za neke deklaracije još od PHP5) uobičajena stvar je imati type hinting i return type u metodama klasa.

Pokušavam da postujem stvari do kojih sam doš’o vlastitom pretragom ili pokupio od iskusni(ji)h saradnika, a koje smatram da su must have danas u PHP-u.