Perché la mia classe è peggiore della gerarchia delle classi nel libro (OOP per principianti)?

11

Sto leggendo Oggetti, modelli e pratica PHP . L'autore sta cercando di modellare una lezione in un college. L'obiettivo è quello di produrre il tipo di lezione (lezione o seminario) e gli addebiti per la lezione a seconda che si tratti di una lezione a prezzo fisso o orario. Quindi l'output dovrebbe essere

Lesson charge 20. Charge type: hourly rate. Lesson type: seminar.
Lesson charge 30. Charge type: fixed rate. Lesson type: lecture.

quando l'input è come segue:

$lessons[] = new Lesson('hourly rate', 4, 'seminar');
$lessons[] = new Lesson('fixed rate', null, 'lecture');

Ho scritto questo:

class Lesson {
    private $chargeType;
    private $duration;
    private $lessonType;

    public function __construct($chargeType, $duration, $lessonType) {
        $this->chargeType = $chargeType;
        $this->duration = $duration;
        $this->lessonType = $lessonType;
    }

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

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

    public function cost() {
        if($this->chargeType == 'fixed rate') {
            return "30";
        } else {
            return $this->duration * 5;
        }
    }
}

$lessons[] = new Lesson('hourly rate', 4, 'seminar');
$lessons[] = new Lesson('fixed rate', null, 'lecture');

foreach($lessons as $lesson) {
    print "Lesson charge {$lesson->cost()}.";
    print " Charge type: {$lesson->getChargeType()}.";
    print " Lesson type: {$lesson->getLessonType()}.";
    print "<br />";
}

Ma secondo il libro, ho torto (sono sicuro che lo sia anch'io). Invece, l'autore ha dato una grande gerarchia di classi come soluzione. In un capitolo precedente, l'autore ha dichiarato i seguenti "quattro pannelli" come il tempo in cui dovrei considerare di cambiare la mia struttura di classe:

L'unico problema che riesco a vedere sono le dichiarazioni condizionali, e anche quelle in modo vago - quindi perché refactarlo? Quali problemi pensate potrebbero sorgere in futuro che non avessi previsto?

Aggiornamento : ho dimenticato di menzionare - questa è la struttura della classe che l'autore ha fornito come soluzione - il modello di strategia :

    
posta Aditya M P 05.04.2012 - 13:03
fonte

5 risposte

19

È divertente che il libro non lo dichiari chiaramente, ma il motivo per cui preferisce una gerarchia di classi su se le istruzioni all'interno della stessa classe sono probabilmente Principio Open / Closed Questa nota regola di progettazione del software afferma che una classe deve essere chiusa alla modifica ma aperta per l'estensione.

Nel tuo esempio, aggiungere un nuovo tipo di lezione significherebbe cambiare il codice sorgente della classe Lesson, rendendolo fragile e soggetto a regressione. Se avessi una classe base e una derivata per ogni tipo di lezione, dovresti semplicemente aggiungere un'altra sottoclasse, che è generalmente considerata più pulita.

Se vuoi, puoi mettere questa regola nella sezione "Dichiarazioni condizionali", tuttavia ritengo che il "cartello" sia un po 'vago. Se le affermazioni sono generalmente solo odori di codice, sintomi. Possono derivare da un'ampia varietà di cattive decisioni di progettazione.

    
risposta data 05.04.2012 - 14:05
fonte
16

Immagino che ciò che il libro si propone di insegnare sia evitare cose come:

public function cost() {
    if($this->chargeType == 'fixed rate') {
        return "30";
    } else {
        return $this->duration * 5;
    }
}

La mia interpretazione del libro è che dovresti avere tre classi:

  • AbstractLesson
  • HourlyRateLesson
  • FixedRateLesson

Dovresti quindi reimplementare la funzione cost su entrambe le sottoclassi.

La mia opinione personale

per casi semplici come quello: no. È strano che il tuo libro favorisca una gerarchia di classi più profonde per evitare semplici dichiarazioni condizionali. Inoltre, con le specifiche di input, Lesson dovrà essere una factory con una spedizione che è un'istruzione condizionale. Quindi con la soluzione del libro avrai:

  • 3 classi anziché 1
  • 1 livello di ereditarietà invece di 0
  • lo stesso numero di istruzioni condizionali

Questa è una maggiore complessità senza ulteriori vantaggi.

    
risposta data 05.04.2012 - 13:35
fonte
7

L'esempio nel libro è piuttosto strano. Dalla tua domanda, l'affermazione originale è:

The goal is to output the Lesson Type (Lecture or Seminar), and the Charges for the lesson depending on whether it is a hourly or fixed price lesson.

che, nel contesto di un capitolo su OOP, significherebbe che l'autore probabilmente vuole che tu abbia la seguente struttura:

+ abstract Lesson
    - Lecture inherits Lesson
    - Seminar inherits Lesson

+ abstract Charge
    - HourlyCharge inherits Charge
    - FixedCharge inherits Charge

Ma poi arriva l'input che hai citato:

$lessons[] = new Lesson('hourly rate', 4, 'seminar');
$lessons[] = new Lesson('fixed rate', null, 'lecture');

vale a dire. qualcosa che viene chiamato codice scritto in modo stringato e che, onestamente, in questo contesto quando il lettore è destinato a imparare OOP, è orribile.

Questo significa anche che se ho ragione circa l'intenzione dell'autore del libro (cioè creare quelle classi astratte ed ereditate che ho elencato sopra), questo porterà a un codice duplicato e piuttosto illeggibile e brutto:

const string $HourlyRate = 'hourly rate';
const string $FixedRate = 'fixed rate';

// [...]

Charge $charge;
switch ($chargeType)
{
    case $HourlyRate:
        $charge = new HourlyCharge();
        break;

    case $FixedRate:
        $charge = new FixedCharge();
        break;

    default:
        throw new ParserException('The value of chargeType is unexpected.');
}

// Imagine the similar code for lecture/seminar, and the similar code for both on output.

Come per i "pannelli":

  • Duplicazione codice

    Il tuo codice non ha duplicati. Il codice che l'autore si aspetta che tu scriva.

  • La classe che sapeva troppo del suo contesto

    La tua classe passa semplicemente le stringhe dall'input all'output e non sa nulla. Il codice atteso d'altra parte può essere criticato per sapere troppo.

  • The Jack of All Trades - Classi che cercano di fare molte cose

    Ancora una volta, stai passando le stringhe, nient'altro.

  • Dichiarazioni condizionali

    C'è una dichiarazione condizionale nel tuo codice. È leggibile e facile da capire.

Forse, infatti, l'autore si aspettava che tu scrivessi un codice molto più refactored rispetto a quello con sei classi sopra. Ad esempio, puoi utilizzare modello di fabbrica per lezioni e addebiti, ecc.

+ abstract Lesson
    - Lecture inherits Lesson
    - Seminar inherits Lesson

+ abstract Charge
    - HourlyCharge inherits Charge
    - FixedCharge inherits Charge

+ LessonFactory

+ ChargeFactory

+ Parser // Uses factories to transform the input into 'Lesson'.

In questo caso, finirai con nove classi, che saranno un perfetto esempio di over-architecture .

Invece, hai finito con un codice pulito, facile da capire, che è dieci volte più breve.

    
risposta data 05.04.2012 - 13:53
fonte
5

Prima di tutto puoi cambiare i "numeri magici" come 30 e 5 nella funzione cost () con variabili più significative :)

E ad essere onesti, penso che non dovresti preoccuparti di questo. Quando avrai bisogno di cambiare classe, la cambierai. Cerca di creare valore, quindi passa alla rifattorizzazione.

E perché pensi di sbagliare? Questa classe non è "abbastanza buona" per te? Perché sei preoccupato per qualche libro;)

    
risposta data 05.04.2012 - 13:25
fonte
3

Non è assolutamente necessario implementare classi aggiuntive. Hai un po 'di problema con MAGIC_NUMBER nella tua funzione cost() , ma il gioco è fatto. Qualsiasi altra cosa è una massiccia sovraingegneria. Tuttavia, sfortunatamente è comune che venga dato un consiglio pessimo: Circle eredita Shape, per esempio. Non è sicuramente efficiente in alcun modo derivare una classe per l'ereditarietà semplice di una funzione. Potresti invece utilizzare un approccio funzionale per personalizzarlo.

    
risposta data 05.04.2012 - 13:41
fonte

Leggi altre domande sui tag