Συμβουλές για καλύτερη σχεδίαση OOP με πραγματικό παράδειγμα

Click4Money · 10 Αυγούστου 2016

Γεια σας, τον τελευταίο καιρό προσπαθώ να βελτιωθώ στον ood προγραμματισμό για καλύτερη συντηρησιμότητα, επεκτασιμότητα και αναγνωσιμότητα στον κώδικα.

Πάρακατω θα σας ποστάρω τον κώδικα μου ο οποίος λειτουργεί άψογα με ένα πραγματικό παράδειγμα. Φανταστείτε ένα σύστημα όπου μπορείς να ανεβάζεις αρχεία διάφορων τύπων (pdf, jpg, excel...) στον σερβερ και να τα αποθηκευεις στην βάση δεδομένων.

Ο κώδικας είναι σε php. Όποια δήποτε συμβουλή ή καλύτερη προσέγγιση στον κώδικα είναι ευπροσδεκτη.

Η base class -> File:

abstract class File
{
    protected $file;
    protected $fileName;
    protected $fileExtension;
    protected $storeFile;
    protected $fileNameWithExtension;

    public function __construct($file, StoreFile $storeFile)
    {
        $this->file = $file;
        $this->fileName = pathinfo($this->file->getClientOriginalName(), PATHINFO_FILENAME);
        $this->fileExtension = pathinfo($this->file->getClientOriginalName(), PATHINFO_EXTENSION);
        $this->storeFile = $storeFile;
        $this->fileNameWithExtension = $this->fileName . '.' . $this->fileExtension;

    }

    abstract protected function getPath();

    abstract function setNewFileNameIfExist();

    public function storeToDB()
    {
        $this->storeFile->toDB($this->fileNameWithExtension);
    }

    public function storeToServer()
    {
        $this->storeFile->toServer($this->file, $this->getPath(), $this->fileNameWithExtension);
    }

}

Interface StoreFile:

interface StoreFile
{
    public function toDB($fileName);
    public function toServer($file, $filePath, $fileName);
}

Παρακάτω θα αναφέρω τις κλάσεις για την αποθήκευση pdf files

StorePDFFile κλάση:

class StorePDFFile implements StoreFile
{
    public function toDB($fileName)
    {
        $pdf = new PDF();
        $pdf->title = $fileName;
        $pdf->save();
    }

    public function toServer($file, $filePath, $fileName)
    {
        Storage::put($filePath . $fileName, File::get($file));

    }
}

και PDF κλάση:

class PDF extends File
{
    public function __construct($file)
    {
        parent::__construct($file, new StorePDFFile());
    }

    public function setNewFileNameIfExist()
    {
        $counter = 0;
        $originalFileName = $this->fileName;
        $allPDF = PDFModel::all();
        foreach ($allPDF as $pdf) {
            if($pdf->title === $this->fileNameWithExtension) {
                while($pdf->title === $this->fileNameWithExtension) {
                    $this->fileNameWithExtension = $originalFileName . '(' . $counter . ')' . '.' . $this->fileExtension;
                    $counter++;
                }
            }
        }
    }

    protected function getPath()
    {
        return 'public/pdf/';
    }
}

Στον client:

        $file = $request->file('pdf');
        $test = new PDF($file);
        $test->setNewFileNameIfExist();
        $test->storeToDB();
        $test->storeToServer();

defacer ρίξε ένα βλέφαρο αν μπορείς

Επεξ/σία 11 Αυγούστου 2016 από Click4Money

paparovic · 10 Αυγούστου 2016

Δεν βλέπω PDFFileServiceFactoryFactory και ανησυχώ.

παπι · 10 Αυγούστου 2016

Βασικα πρωτα γραψτο ετσι οπως θα το εγραφες χωρις κλασεις. Ωστε να δεις τι interface θελεις, και μετα το κανεις οοπ με βαση το interface που θες.

Αλλιως θα εχεις το πανω αποτελεσμα, πχ new pdfservice(new pdf????

Ή εκει που λες toDb και βαζεις name, γιατι να βζλεις name?

Click4Money · 11 Αυγούστου 2016

Ευχαριστώ για τις απαντήσεις. paparovic αν μπορείς να είσαι λίγο πιο συγκεκριμένος, δεν καταλαβαίνω τι εννοείς.

Έχω ενημερώση τον κώδικα με μια αρκετά καλύτερη προσέγγιση πιστεύω.

paparovic · 11 Αυγούστου 2016

paparovic αν μπορείς να είσαι λίγο πιο συγκεκριμένος, δεν καταλαβαίνω τι εννοείς.

Site: http://discuss.joelonsoftware.com/?joel.3.219431.12

defacer · 29 Αυγούστου 2016

defacer ρίξε ένα βλέφαρο αν μπορείς

ΟΚ... λοιπόν, η εντύπωση που μου δίνει είναι ότι το έγραψε κάποιος που έχει μια Α εμπειρία από χρήση καλά σχεδιασμένων classes, αλλά δεν έχει εμπειρία στο σχεδιασμό και kind of πετάει randomly μακαρόνια στον τοίχο να δει τι θα κολλήσει. Μη το παίρνεις στραβά, θεωρώ τον εαυτό μου ικανό να έγραφε τέτοιο κώδικα και μετά από αρκετά χρόνια ενασχόλησης.

Το βασικό θέμα στο OOD είναι ότι πρέπει να ξέρεις τι κάνεις. Για να ξέρεις, πρέπει να μπορείς να το φανταστείς να δουλεύει. Αλλά για να μπορείς να το κάνεις αυτό χωρίς να ξέρεις ήδη πώς γίνεται, πρέπει είτε να το έχεις δει από κώδικα τρίτων είτε να έχεις λιώσει στην ανάλυση και τη μελέτη (μη ρεαλιστικό στην πλειοψηφία των περιπτώσεων).

Οπότε συμπερασματικά μυρίζω ότι έχεις δει ωραία πράγματα να γίνονται και προσπαθείς (ανεπιτυχώς προς το παρόν) να τα κάνεις emulate. No worries, γι' αυτό είμαστε εδώ.

Για να μη πω ξεκινάμε από το μηδέν, ας αρχίσουμε από αυτό που έχεις. Πρώτα μερικές ερωτήσεις:

Τι αντιπροσωπεύει η class File? Πιο συγκεκριμένα, γιατί έχει method setNewFileNameIfExist()? Η ύπαρξη και μόνο αυτής της μεθόδου δε μας λέει ότι η File δεν καταφέρνει να αντιπροσωπεύει ένα αρχείο όπως λέει και τ' όνομά της, αλλά αντίθετα μπλέκεται στα χωράφια μιας κάποιας διαδικασίας;
Κατ' επέκταση, σκέψου το ενδεχόμενο να θέλεις αύριο να αποθηκεύσεις PDF με new file names όπου το νούμερο μπαίνει με τη μορφής [Χ]. Και μεθαύριο με τη μορφή -Χ- . Και όλα αυτά δυναμικά, στην ίδια εφαρμογή. Τι αποτέλεσμα θα έχει αυτό όσον αφορά τις classes σου; Φαίνεται λογικό αυτό το αποτέλεσμα;
Γιατί η File έχει dependency (απαιτεί στον constructor) ένα StoreFile αντί να είναι το dependency ανάποδα? Τι βγάζει καλύτερο νόημα;
Γιατί η File έχει abstract protected getPath()? Τι path είναι αυτό; Γιατί δεν είναι το path μέρος της StoreFile αφού τελικά το μόνο που κάνεις (και που θα κάνεις, όσο μπορεί να προβλέψει κανείς) είναι να περνάς την τιμή σε μέθοδο της StoreFile?

For starters φτάνουν αυτά νομίζω.

Θα σε βοηθήσει πάρα πολύ για όλες τις classes σου να γράφεις σε ικανοποιητικό βαθμό επεξηγηματικά phpdoc. Θα δεις ότι προσπαθώντας να το κάνεις θα δυσκολεύεσαι στην αρχή πάρα πολύ να γράψεις κάτι που βγάζει νόημα. Αυτό σημαίνει σχεδόν πάντα ότι έχεις σχεδιάσει την class/method λάθος, γιατί τα πράγματα που είναι καλά σχεδιασμένα είναι και εύκολο να τα εξηγήσεις συνοπτικά.

Alithinos · 29 Αυγούστου 2016

Αφού η StorePDFFile έχει μονάχα 2 μεθόδους, και μάλιστα μεθόδους σχετικές με PDF, γιατί δεν τις βάζεις αυτές τις μεθόδους απ' ευθείας στη κλάση PDF ?

Το σκεπτικό είναι ότι η κάθε κλάση αντιπροσωπεύει ένα τύπο από κάτι, και οι ιδιότητες και η συμπεριφορά του 'κάτι' είναι μέσα στη κλάση του. Τα βάζεις μέσα για να ξέρεις που να τα βρεις όταν τα χρειαστείς, για να είναι ότι σχετικό με ένα αντικείμενο σε ένα σύνολο που αντιπροσωπεύει το αντικείμενο. Το σύνολο αυτό, είναι η κλάση, και τα μέλη απ' τα οποία αποτελείται.

Άρα ο κατακερματισμός αυτός που κάποιες συναρτήσεις του PDF είναι στη κλάση PDF, και άλλες σε άλλη κλάση, είναι αντιδεοντολογικός. Γιατί και οι μεν και οι δεν αφορούν το ίδιο πράγματα, το PDF.

defacer · 29 Αυγούστου 2016

Αφού η StorePDFFile έχει μονάχα 2 μεθόδους, και μάλιστα μεθόδους σχετικές με PDF, γιατί δεν τις βάζεις αυτές τις μεθόδους απ' ευθείας στη κλάση PDF ?

Το σκεπτικό είναι ότι η κάθε κλάση αντιπροσωπεύει ένα τύπο από κάτι, και οι ιδιότητες και η συμπεριφορά του 'κάτι' είναι μέσα στη κλάση του. Τα βάζεις μέσα για να ξέρεις που να τα βρεις όταν τα χρειαστείς, για να είναι ότι σχετικό με ένα αντικείμενο σε ένα σύνολο που αντιπροσωπεύει το αντικείμενο. Το σύνολο αυτό, είναι η κλάση, και τα μέλη απ' τα οποία αποτελείται.

Άρα ο κατακερματισμός αυτός που κάποιες συναρτήσεις του PDF είναι στη κλάση PDF, και άλλες σε άλλη κλάση, είναι αντιδεοντολογικός. Γιατί και οι μεν και οι δεν αφορούν το ίδιο πράγματα, το PDF.

Αυτό είναι πολύ επιφανειακή αντιμετώπιση και δε θα σε πάει πολύ μακριά.

Πες ότι έβαλε τις μεθόδους στην File καρφωτές.

Η class θα έχει τώρα public methods setNewBlahBlah, storeToServer και storeToDB, να μην αναφέρω καν τη getPath. Φανερά είναι μια class ο σκοπός της οποίας είναι να αποθηκεύει αρχεία (και έτσι θα χρησιμοποιηθεί), όχι μια η οποία αντιπροσωπεύει ένα αρχείο (πιθανότατα έτσι θα δημιουργούνται instances). Αυτό είναι κακός σχεδιασμός.
Το πρόβλημα που ανέφερα παραπάνω σχετικά με το τι θα γίνει αν αποφασίσεις να αλλάξεις για κάποια αρχεία το πώς βρίσκεις new name παραμένει.

Εγώ στον κώδικα που δόθηκε βλέπω τα εξής ξεχωριστά πράγματα:

την ανάγκη αντιπροσώπευσης ενός αρχείου ως κάποια τιμή (ίσως και απλό string για αρχή?)
την ανάγκη "αποθήκευσης" του αρχείου σε κάποιο μέσο
την ανάγκη να ξεκαθαρίσουμε τι θα γίνει αν το όνομα του αρχείου έρχεται σε σύγκρουση με κάποιο ήδη υπάρχον

Για το πρώτο ας πούμε KISS και YAGNI και ας το πάρουμε ως string στην αρχή.

Για το δεύτερο θα μας καλύψει το εξής:

interface FileStorageInterface
{
    public function store($filePath);
}

Για το τρίτο:

interface FileNameConflictResolutionStrategyInterface
{
    public function getUnconflictingFileName($conflictingFileName);
}

Εδώ έχουμε κλασική περίπτωση strategy pattern με την οποία ξεμπερδεύουμε με χαζά methods. Η ιδέα είναι ότι θα έχουμε κάπου κώδικα αποθήκευσης που θα κάνει το εξής:

if (isConflictingFileName($fileName)) {
    // κάνε ο,τι θες
    $fileName = $conflictResolutionStrategy->getUnconflictingFileName($fileName);

    if (isConflictingFileName($fileName)) { // υπόψη, μπορεί να μας είπε ψέματα
        throw new RuntimeException("μπλα μπλα μας είπαν ψέματα");
    }
}

// και τώρα σώσε όντας σίγουρος πως δεν υπάρχει conflict

Αυτός ο κώδικας όμως θα ήταν βασικά μέσα στην FileStorageInterface::store(), στην οποία δεν έχουμε πρόβλεψη για να βρίσκεται κάποιο $conflictResolutionStrategy πρόχειρο. Αυτό μπορεί να λυθεί εξίσου καλά (ανάλογα την περίπτωση) με δύο τρόπους, είτε βάζουμε άλλο ένα όρισμα είτε το interface θα γίνει implemented από classes που θα κρατάνε ένα FileNameConflictResolutionStrategyInterface σαν member.

Αν τα βάλεις κάτω θα δεις ότι έτσι έχω πετύχει να κρατήσω τελείως ξεχωριστά μεταξύ τους τα τρία κομμάτια του παζλ που εντόπισα.

Click4Money · 30 Αυγούστου 2016

@defacer Σε ευχαριστώ πολύ για την απάντηση. Θα μελετήσω καλά όσα μου είπες και θα επανέλθω. Ήδη τα ερωτήματα που μου έθεσες με βοήθησας πάραπολύ... .

ΥΓ. Δουλέυω σε εταιρεία ως full-stack developer. Στην εταιρεία δεν υπάρχει κάποιος έλεγχος στον κώδικα, απλά κάνω τα πράματα να δουλεύουν. Ο λόγος που ξεκίνησα να προσπαθώ στην OOD είναι γιατί όπως είπες έχω δει ωραία open source project που χρισιμοποιουν αυτές τις τακτικές και θέλω να βελτιωθώ και εγω πάνω σε αυτό το κομμάτι. Στην συνέχεια όταν νιώσω έτοιμος θα ήθελα να προχωρήσω και με unit testing.

Click4Money · 31 Αυγούστου 2016

Μετά από μεγάλη προσπάθεια, αφού διάβασα 5 τουλάχιστον φορές την απάντηση από τον defacer, με πολλές και διαφορετικές σκέψεις για τον σχεδιασμό αυτόυ του προβλήματος (συνεχώς στριφογύριζα την όλη ιδέα στο μυαλό μου), κατέληξα στην παρακάτω υλοποίηση:

Η κλαση PDFModel που χειρίζομαι στον κώδικα είναι για την αποθήκευση και την ανάκτηση των pdf στην βάση δεδομέων.

Κλάση File:

class File
{
    private $file;

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

    public function getFile()
    {
        return $this->file;
    }

}

Κλάση PDF

class PDF
{

    const FILEPATH = 'public/pdf/';
    private $file;
    private $unconflictingFileName;
    public $storage;

    public function __construct($file)
    {
        $this->file = $file;
        $this->unconflictingFileName = (new FileConflictResolution(new PDFModel()))->getUnconflictingFileName($this->getFileName(), $this->getFileExtension());
        $this->storage = new FileStorage(new PDFModel(), $this->file, $this->getUncoflictingName(), self::FILEPATH);
    }

    private function getFileName()
    {
        return pathinfo($this->file->getClientOriginalName(), PATHINFO_FILENAME);
    }

    private function getFileExtension()
    {
        return pathinfo($this->file->getClientOriginalName(), PATHINFO_EXTENSION);
    }

    private function getFileNameWithExtension()
    {
        return $this->getFileName() . $this->getFileExtension();
    }

    private function getUncoflictingName()
    {
        return $this->unconflictingFileName;
    }

}

Interface FileNameConflictResolutionStrategyInterface

interface FileNameConflictResolutionStrategyInterface
{
    public function isConflictingFileName($fileName);
    public function getUnconflictingFileName($conflictingFileName, $fileExtension);
}

Κλάση FileConflictResolution

class FileConflictResolution implements FileNameConflictResolutionStrategyInterface
{

    private $fileModel;

    public function __construct($fileModel)
    {
        $this->fileModel = $fileModel;
    }


    public function isConflictingFileName($fileName)
    {
        foreach ($this->fileModel->all() as $file) {
            if ($file->title === $fileName) {
                return true;
            }
        }
        return false;
    }

    public function getUnconflictingFileName($conflictingFileName, $fileExtension)
    {
        $counter = 0;
        $unConflictingFileName = $conflictingFileName . '.' . $fileExtension;
        while ($this->isConflictingFileName($unConflictingFileName)) {
            $unConflictingFileName = $conflictingFileName . '(' . $counter . ')' . '.' . $fileExtension;
            $counter++;
        }
        return $unConflictingFileName;
    }
}

Interface FileStorageInterface

interface FileStorageInterface
{
    public function toServer();
    public function toDB();
}

Κλάση FileStorage

class FileStorage implements FileStorageInterface
{

    private $fileModel;
    private $file;
    private $uncoflictingFileName;
    private $filePath;

    public function __construct($fileModel, $file, $uncoflictingFileName, $filePath)
    {
        $this->fileModel = $fileModel;
        $this->file = $file;
        $this->uncoflictingFileName = $uncoflictingFileName;
        $this->filePath = $filePath;
    }

    public function toServer()
    {
        //save file to server. εδώ θα χρειαστώ το filePath

    }

    public function toDB()
    {
        $this->fileModel->title = $this->uncoflictingFileName;
        $this->fileModel->save();
    }
}

Στον client:

        $file = $request->file('pdf');
        $file = new File($file);

        $pdfFile = new PDF($file->getFile());
        $pdfFile->storage->toDB();
        $pdfFile->storage->toServer();

defacer · 31 Αυγούστου 2016

Μετά από μεγάλη προσπάθεια, αφού διάβασα 5 τουλάχιστον φορές την απάντηση από τον defacer, με πολλές και διαφορετικές σκέψεις για τον σχεδιασμό αυτόυ του προβλήματος (συνεχώς στριφογύριζα την όλη ιδέα στο μυαλό μου), κατέληξα στην παρακάτω υλοποίηση:

Πού είναι το phpdoc?

Συνεχίζω τα σχόλια από την τελευταία version.

Στην αρχική μου πρόταση είχα μία μόνο ευθύνη πάνω στο ConflictResolutionStrategyInterface: να βρει ένα καινούριο unconflicting όνομα (όχι απαραίτητα ε: μπορεί 100% legitimately να υπάρχει ένα implementation που απλά κάνει throw επειδή "αν φτάσαμε στη φάση που έχουμε conflict κάτι πήγε ήδη στραβά"). Εδώ τώρα του έχεις δύο ευθύνες: να δει επιπλέον αν το τάδε όνομα είναι conflicting. Αλλά προφανώς οι δύο αυτές ευθύνες είναι τελείως άσχετες μεταξύ τους. Άλλο πράγμα το "conflicting" σε ένα case sensitive filesystem, άλλο σε case insensitive, άλλο όταν γράφουμε blobs σε μια database, etc -- αλλά σε όλες τις περιπτώσεις, το πώς πάμε σε unconflicting μπορεί να είναι η ίδια ακριβώς διαδικασία. Είναι λοιπόν ορθογώνια μεταξύ τους πράγματα και δε θα πρέπει να είναι υποχρεώσεις της ίδιας class.

Συγκεκριμένα, το "detect conflict" μέρος θα πρέπει να είναι λογικά μέρος του FileStorageInterface (όχι απαραίτητα public στο interface, μπορεί κάλλιστα να είναι private ή protected σε κάποια υλοποίηση). Αφού το interface αντιπροσωπεύει τον ίδιο το μηχανισμό του storage, δεν είναι λογικό η υλοποίηση του interface να είναι ο αρμόδιος να απαντήσει αν κάποιο όνομα είναι conflicting με κάτι που ήδη υπάρχει στο storage?

Επίσης άσχετα μεταξύ τους είναι τα δύο πράγματα που κάνει το FileStorage. Αντί να γίνει έτσι το λογικότερο θα ήταν να έχουμε δύο ξεχωριστές υλοποιήσεις του και η κάθε μία να κάνει save() στο αντίστοιχο μέσο.

Ο κώδικας στον οποίο θα ήθελά εγώ να καταλήξω είναι σε πρώτη φάση περίπου ο εξής:

$file = $request->file('pdf');

$crs = new AppendIncrementingCounterFileNameConflictResolutionStrategy(); // λολ
$lfs = new LocalFilesystemStorage($crs);
$dfs = new DatabaseFileStorage(new PDFModel(), $crs);

$lfs->store($file);
$dfs->store($file);

Το μόνο πράγμα που δεν είναι καλό εδώ είναι το ξεκάρφωτο new PDFModel(), που δε φαίνεται ούτε πώς προκύπτει (γιατί hardcoded PDF αδερφέ, κι αν μετά στο store() σου δώσω κάτι που δεν είναι PDF?) ούτε πώς χρησιμοποιείται (θα κάνω findAll() και ->title, από πού κι ως πού μπορεί να γίνει documented αυτό???). Και μόνο η σωστή αντιμετώπιση αυτής της λεπτομέρειας είναι ολόκληρη κουβέντα, αλλά προς το παρόν δεν είναι "χειρότερο" από το hardcoded new PDF() οπότε ΟΚ.

Τα isConflictingFileName() και οτιδήποτε έχει να κάνει με paths κλπ είναι εσωτερικά στη StorageInterface::store(). Για παράδειγμα στη LocalFilesystemStorage:

public function store(SplFileInfo $file)
{
    $storageRoot = $this->getStorageRootFor($file);
    $storedFileName = $this->combinePath($storageRoot, $file->getFilename());
    if (file_exists($storedFileName)) {
        // τα είπαμε νωρίτερα
    }

    file_put_contents($storedFileName, "το έσωσα!"); //whatever
}

private function getStorageRootFor(SplFileInfo $file)
{
    switch (strtolower($file->getExtension())) {
        case 'pdf': return 'public/pdf';
        default: throw new LogicException("δεν ξέρω τι να το κάνω αυτό");
    }
}

private function combinePath(...$pathComponents)
{
    return implode('/', array_filter($pathComponents, function($s) { return strlen($s); }));
}

Αν τώρα θες αυτή τη φάση με το filename conflict resolution να τη χρησιμοποιήσεις πολλές φορές, θα μπορούσες για επαναχρησιμοποίηση να κάνεις π.χ.

abstract class AbstractStorage implements StorageInterface
{
    private $filenameConflictResolutionStrategy;

    // constructor...

    public final function store(SplFileInfo $file) // final is part of the design
    {
        $storageName = $this->getStorageFileName($file);
        $this->storeImplementation($file, $storageName);
    }

    protected final function getStorageFileName(SplFileInfo $file) // final as above
    {
        $fileName = $this->getPreferredNameForStorage($file);
        if ($this->isConflictingFilename($storageName)) {
            // etc etc
        }

        return $fileName;
    }

    protected function getPreferredNameForStorage(SplFileInfo $file)
    {
        return $file->getFilename();
    }

    abstract protected function isConflictingFilename($fileName); // OXI SplFileInfo πλέον

    abstract protected function storeImplementation(SplFileInfo $file, $storageName);
}

Πάμε τώρα για μερικά τεστάκια με τα οποία καταλαβαίνεις ότι αυτό το design είναι "καλό" (ή τέλος πάντων "καλύτερο"):

Πες ότι αποφάσισες να προσθέσεις καινούριο επιπλέον είδος storage, π.χ. σε Redis ή στο PHP session ή whatever. Προφανώς θα πρέπει να γράψεις κανα δυο γραμμές ακόμα στον κώδικα που καλείς τις store() ή αντίστοιχα. Αλλά με αυτό το design, πόσες από τις ήδη υπάρχουσες classes θα πρέπει να πειράξεις για να αποκτήσεις το νεο feature? Μηδέν. Πράγμα που είναι αν το καλοσκεφτείς η σωστή απάντηση.

Πες ότι εκτός από PDF θέλεις να βάλεις και κάτι άλλο τελείως καινούριο. Πόσες classes θα πρέπει να πειράξεις για να δουλέψει; Μόνο τη LocalFilesystemStorage, και μέσα σε αυτή μόνο μία method, και αυτή η method δεν κάνει τίποτα άλλο από το να επιστρέφει paths. Αυτό είναι το καλύτερο δυνατό αποτέλεσμα από την άποψη της ελάχιστης αναστάτωσης, και επιπλέον είναι λογικό να ξέρει η LFS πού ακριβώς θα αποθηκεύει το κάθε αρχείο (δεν είναι ανάγκη να το ξέρει η ίδια, μπορεί να έχει ένα dependency που η δουλειά του είναι μόνο να ξέρει αυτά τα πράγματα, να υλοποιεί δηλαδή την getStorageRootFor -- αλλά ας πούμε YAGNI).

Γενικά το design αποσκοπεί στο να μπορούμε εμείς οι soft humans να έχουμε στο μυαλό μας πώς λειτουργεί το σύστημα, και να μπορούμε να κάνουμε επεμβάσεις σε αυτό με σιγουριά και ασφάλεια.

Γι' αυτό σου λέω γράφε phpdoc (αν δε μπορείς να εξηγήσεις το design σου, δεν έχεις κάνει design) και έτσι προκύπτουν και τα παραδείγματα ("πες τώρα ότι θέλουμε να...").

Κλείνοντας: να αποκτήσεις τη συνήθεια να μην κάνεις ποτέ new Anything() πουθενά μέσα σε μια class, εκτός αν ο σκοπός για τον οποίο τη χρησιμοποιείς είναι ακριβώς να σου κάνει ένα καινούριο instance από κάτι. Θα αναγκαστείς τότε ως caller να περνάς οτι σου χρειάζεται σε constructor arguments (dependency injection) και μέσα στις classes σου να λειτουργείς ως client αυτών των dependencies (inversion of control). Αυτό με τη σειρά του θα σε ωθήσει πολύ δυνατά να χρησιμοποιήσεις αρκετά βασικά design patters, ακόμα και χωρίς να ξέρεις απαραίτητα στην αρχή πως λέγεται αυτό που κάνεις. Και μετά θα έρθει μια μέρα που θα ξυπνήσεις και θα κλικάρουν κομμάτια του παζλ, θα καταλάβεις τι επιπτώσεις έχει ο τρόπος με τον οποίο σχεδιάζεις τα πράγματα στον τρόπο με τον οποίο χρησιμοποιούνται και θα πεις

ΥΓ: Ένα άλλο εξαιρετικό για εκπαιδευτικούς σκοπούς που μπορείς να κάνεις είναι το εξής: κάνε μια μικρή εισαγωγή στο unit testing έτσι να μάθεις τα βασικά και να μπορείς να γράψεις μερικά test. Μετά, γράψε μια σειρά από unit tests που θα ήθελες να έχεις για να είσαι σίγουρος ότι ο κώδικας "δουλεύει καλά". Για παράδειγμα:

Αν πας να σώσεις στη db και υπάρχει conflict στο filename, το conflict εντοπίζεται σωστά.
Αν πας να σώσεις στo filesystem και υπάρχει conflict στο filename, το conflict εντοπίζεται σωστά.
Αν υπάρχει conflict, λύνεται με το σωστό τρόπο (βρίσκουμε το επόμενο unconflicting filename με (Ν) από πίσω).

Δοκίμασε τέλος να γράψεις αυτά τα unit tests για τον κώδικα που δίνω και για κάποια άλλη εκδοχή και δες τι θα συμβεί.

Update: χαίρομαι πολύ που το βρήκατε χρήσιμο παιδιά! Έκανα λίγα edits σε μερικά σημεία για να τα πω λίγο καλύτερα, στο τμήμα που είναι αφού τελειώσει ο κώδικας. Επίσης είδα το post του dios που μιλάει για abstractions, και δε μπορώ να μη παραθέσω αυτό το εξαιρετικό quote του Dijkstra...

"The purpose of abstraction is not to be vague, but to create a new semantic level in which one can be absolutely precise."

Σε αυτό το context δείτε λίγο στο προηγούμενο post εκεί που λέω στον κώδικα που δόθηκε βλέπω τα εξής ξεχωριστά πράγματα. Νομίζω ότι το παράδειγμα ήταν σε πολύ καλό επίπεδο για να φαίνεται η αξία της συγκεκριμένης επιλογής abstractions στην πράξη.

Έχοντας κάνει ένα κατάλληλο για την περίπτωση σετ από abstractions (χωρίς ακόμα να μπαίνει γλώσσα προγραμματισμού μέσα), το design στη συνέχεια "γράφεται μόνο του", και στην πορεία της υλοποίησης προκύπτουν με φυσικό τρόπο design patterns. Των οποίων αυτή είναι εξάλλου η δουλειά: να υλοποιούν σε επίπεδο γλώσσας προγραμματισμού (πιο χαμηλά δηλαδή και από το αλγοριθμικό) το εκάστοτε design.

Στη συγκεκριμένη περίπτωση έχουν χρησιμοποιηθεί Strategy και Template Method. Θα μπορούσε κάποιος να καταλήξει σε αυτό τον κώδικα because it feels right χωρίς να ξέρει το θεωρητικό μέρος, αν και πάντα είναι καλύτερα να ξέρεις τη θεωρία.

Επεξ/σία 5 Σεπτεμβρίου 2016 από defacer

Click4Money · 31 Αυγούστου 2016

@defacer 10000000 ευχαριστώ για τον χρόνο σου και για όλη αυτή την βοήθεια που μου έδωσες.

.......... .......... Ώρα για μελέτη και εξάσκηση τώρα .......... ..........

dios231 · 4 Σεπτεμβρίου 2016

Μετά από αυτό το legendary post toy defacer, προτείνω να διαβάσεις και αυτό το article από τον mark seemann http://blog.ploeh.dk/2010/12/03/Towardsbetterabstractions/.

Καλό θα ήταν να διαβάσεις και τα reference links που έχει μεσα για να καταλάβεις καλύτερα τι προσπαθεί να λύσει με αυτό το article.

defacer · 5 Σεπτεμβρίου 2016

Bump, έκανα ένα υπολογίσιμο update παραπάνω αφού ξαναδιάβασα τη συζήτηση και τα reactions.

kaliakman · 10 Σεπτεμβρίου 2016

Συγγνώμη για το fork αλλά είπα να μην ανοίξω καινούριο θέμα μιας και είναι της ίδιας φύσεως.

Παίζω με μια προσομοιώση περιβάλλοντος όπου υπάρχουν φυτά και ζώα.

Εδώ φαίνεται και το UML από το IDEA:

Είμαι στην φάση της μοντελοποίησης και το θέμα μου είναι οτί σχεδόν όλες τις μεταβλητές στις Animal και Plant τις έχω protected μιας και σε κάθε διαφορετικό τύπο(algae,pine κτλ) τις θέτω γιατί διαφέρουν.

But it just doesn't feel right.

Εννοώ οτί το θέμα είναι να γίνεται όσο καλύτερο encapsulation μπορώ ωστέ να είναι maintainable ο κώδικας αλλά εδώ έχω *όλες* τις μεταβλητές να κατεβαίνουν 2 κλάσεις κάτω.

Η ενναλακτική που σκέφτηκα είναι με setters αλλά ουσιαστικά θα κάνει το ίδιο πράγμα.

Υπερβάλλω ή μπορεί να γίνει κάτι άλλο;

Σύνδεση

Συμβουλές για καλύτερη σχεδίαση OOP με πραγματικό παράδειγμα

Προτεινόμενες αναρτήσεις

Click4Money

paparovic

παπι

Click4Money

paparovic

defacer

Alithinos

defacer

Click4Money

Click4Money

defacer

Click4Money

dios231

defacer

kaliakman

Δημιουργήστε ένα λογαριασμό ή συνδεθείτε για να σχολιάσετε

Δημιουργία λογαριασμού

Σύνδεση

Σύνδεση