Autor Zpráva
HugoFalkW
Profil
Ahoj,

snažím se naučit a hlavně pochopit PHP OOP. Zkouším to na jednoduchých příkladech a byl bych moc vděčný za to, že mi pomůžete.

Vložím sem ukázku galerie, kterou jsem včera vytvořil a jejíž stav se bude průběžně měnit. Vás prosím o to, aby jste mi řekli, co je v kódu špatně po stránce OOP - prosím berte pouze v potaz vložený kód, určitě existuje mnoho jiných řešení jak to udělat. Mě jde spíše o to, jestli jdu správným směrem, jestli mám správně vyjímky, metody apod. Děkuji

class Galerie {
    private $cols = 5;
    private $folder;
    private $images = array();
    private $ignore = array(".","..","Thumbs.db","DS_Store",".htaccess");
    private $excluded = array();
    private $output;

    public function __construct($folder) {
        $this->folder = $folder;
    }
    public function setFolder($folder) {
        $this->folder= $folder;
    }
    private function scanDirectory() {
        if(is_dir($this->folder)) {
            foreach(scandir($this->folder) as $key=>$value) {
                if(in_array($value, $this->ignore)) {
                    unset($key);
                    $this->excluded[] = $value;
                } else {
                    $this->images[] = $value;
                }
            }
        } else {
            throw new Exception("The Folder has not valid.", 1);
            
        }
    }
    public function getFolder() {
        return $this->folder;
    }

    public function addIgnore($ignore) {
        $this->ignore[] = $ignore;
    }
    public function getIgnore() {
        return $this->ignore;
    }
    public function getImages() {
        try{
        $this->scanDirectory();
        if(is_scalar($this->images)) 
                throw new Exception("Program error: In the folder there are not no pictures.", 1);
        return $this->images;
        } catch (Exception $e) {
            echo "Program error: " . $e->getMessage();
        }
    }

    public function showImages() {
        try {
            $this->scanDirectory();
            if(is_scalar($this->images)) 
                throw new Exception("Program error: In the folder there aren't no pictures.", 1);
            $counter = 0;
            $this->output .= "<TABLE ID=\"galerie\"><TR>";
            foreach($this->images as $img) {
                $counter++;
                $this->output .= "<TD><a href=\"".$this->folder."/".$img."\"><img src=".$this->folder."/nahledy/".$img." width=\"120\" height=\"120\" border=\"0\"></TD>";
                if(!($counter % $this->cols))
                    $this->output .= "</TR><TR>";
            }
            $this->output .= "</TR></TABLE>";
            return $this->output;

        } catch (Exception $e){
            echo "Program error: " . $e->getMessage();
        }
    }
}

$galerie = new Galerie("./tapety");
$galerie->addIgnore("nahledy");
echo $galerie->showImages();
?>
<style>
table tr td img {
    border: 1px solid black;
    padding: 3px;
    box-shadow: gray 2px 3px 10px;
    margin: 3px;
}
<style>



Moderátor Joker: Nastavil jsem správný zvýrazňovač kódu
Petr Ká
Profil
No aj osobně si myslím, že na začátečníka v OOP toto nevypadá zle :)
Jen možná scanDirectory() by mohlo být zavoláno už při _construct a nevolat ho několikrát (max, při změně adresáře/nastavení nového ignoru a pod.)
Filip Sivák
Profil
Toto nevypadá jako správné OOP. OOP není jen o tom, že napíšeš třídu v PHP. Ta třída musí mít jisté vlastnosti.

Zásadní problém je v metodě showImages, mícháš HTML kód a PHP třídu, to nedělej. Už vůbec ne tak, že bys ukládal výstup do nějaké proměnné, určitě sis všiml, že ti trvalo dlouho to napsat :). Lepší bude si vytvořit soubor galerieTemplate.php, který může vypadat například takto:
<?php 
    // nastaveni si dam nahoru do konstant, at nemusim nic hledat dole v kodu
    define("FOLDER", "./tapety");
    define("IGNORE", "nahledy");

    $galerie = new Galerie( FOLDER );
    $galerie->addIgnore( IGNORE );
?>

<table id="galerie">
    <tr>
        <?php // pouzivam foreach s dvojcekou pro vetsi citelnost v sablone ?>
        <?php foreach($images as $img): ?>
                $counter++;
        ?>
            <td>
                <?php // znacka <?= slouzi jako prikaz pro vypis ?>
                <a href="<?= FOLDER."/".$img">
                    <img src="<?= FOLDER."/nahledy/".$img ?>" 
                        width="120" 
                        height="120" 
                        border="0">
            </td>
        <?php endforeach; ?>
    </tr>
</table>
V příkladu není doimplementovaný ten counter. BTW na galerii použij raději divy s floatem, než tabulku.

Další problémy:
- český název třídy, používej angličtinu (v budoucnosti se to beztak budeš muset odnaučit, odnauč se teď)
- používej protected namísto private, jedna z nejsilnějších vlastností OOP je dědění (pokud používáš OOP bez dědění, děláš něco špatně), protected metody se dají dědit, private ne. Private používej tam, kde nechceš, aby tu metodu někdo přepisoval v potomkovi.
- když zavolám getImages() a následně showImages() bude se adresář scanovat 2x, řeší se to takto:

class ClassWithCachedProperty {
    protected $cached = null;
    
    public function getCached() {
        // pokud "cached" nikdo nenahral, tak ho nahraj
        if($this->cached == null) {
                        // nacte do property cached obsah
            $this->loadCached();
        }
        return $this->cached;
    }
}

- vytvoř si vlastní třídy pro exceptiony tím, že podědíš RuntimeException nebo Exception, přečti si o tom více
- HTML tagy se píší malými písmeny (přeci nechceš aby na tebe zdroják "křičel" :)).
- gettery & settery nemáš všechny. Je dobrou praktikou je ve zdrojáku dávat až někam dolů a metody které doopravdy něco dělají mít výše. Nečekává se, že by getter, nebo setter dělal něco jiného, než vracel (případně kontroloval cachování, viz výše). getImages() nahrává obrázky, to není úplně ideální, protože programátor o tom z názvu neví. Bylo by třeba mu to říct třeba komentářem. Je mi jasné, že programuješ pro sebe, ale ptáš se na správné OOP a tohle k tomu patří.

Vaše odpověď

Mohlo by se hodit

Odkud se sem odkazuje


Prosím používejte diakritiku a interpunkci.

Ochrana proti spamu. Napište prosím číslo dvě-sta čtyřicet-sedm: