Richiesta di feedback sul mio design OO

Dopo aver letto le risposte postate a questa domanda, ho deciso di modificare il mio design. Ecco come appare ora (semplificato):

Nel progetto precedente, Melody e Progression facevano quasi la stessa cosa, tranne che stavano giocando elementi diversi: Note s o Chord s rispettivamente. Creazione di duplicati.

Mi sono reso conto che posso migliorare il design dando a Note e Chord un titolo comune: Playable . Di quanto posso avere una classe più generale Sequence che riproduce Playable s.

MelodyGenerator e ProgressionGenerator quindi entrambi istanziano lo stesso tipo di oggetto, a Sequence , solo che lo compongono con un diverso tipo di elementi: o Note s o Chord s.

Inoltre, ho rinunciato al tipo di oggetto ricorsivo che avevo nel progetto precedente, in cui un StructuredMelody può contenere un Melody . Non più tipi diversi di melodie e progressioni: tutto è solo un Sequence .

Le diverse sottoclassi di MelodyGenerator e ProgressionGenerator implementano semplicemente algoritmi diversi per produrre il prodotto Sequence .

Il StructuredGenerator s contiene un riferimento a un generale Generator , il che significa che hanno ancora l'opzione di utilizzare un altro StructuredGenerator internamente. Dipende da loro Tuttavia, il loro valore predefinito è di utilizzare un SimpleGenerator internamente.

Sono riuscito a migliorare il design trovando i luoghi in cui è possibile eliminare la duplicazione dando un titolo più generale alle cose: I.e. Ho capito che Progression e Melody sono praticamente la stessa cosa, quindi ho astratto le cose chiamandole entrambe a Sequence .

Inoltre, la realizzazione del modello composito nelle melodie e nelle progressioni non era necessaria, ha contribuito a chiarire il design.

Grazie per il tuo feedback.

Quelle sono davvero sottoclassi? Non penso che tu abbia bisogno di tutte quelle sottoclassi.

Come hai già indicato:

SimpleMelody is composed of an array of notes. Invoking play on it plays these notes one after the other. StructuredMelody is composed of an array of Melody objects. Invoking play on it plays these Melodies one after the other.

In altre parole, SimpleMelody è una matrice di Array of Notes, non una "è una". StructuredMelody è una matrice di oggetti melodici, non un'elaborazione di Melody.

Sono solo raccolte, in altre parole. Se si desidera racchiudere tali raccolte nelle proprie classi e assegnare loro proprietà o metodi aggiuntivi, utilizzare la composizione, non l'ereditarietà.

Non c'è niente di sbagliato nell'avere classi di generatore one-for-one. Se questo ti mette a disagio, metti i metodi statici di fabbrica nelle classi reali, come in

var melody = Melody.Create(foo);

In generale, penso che tu stia creando molta astrazione senza troppi benefici. Basta attenersi alle classi Note, Melody e Collection in un primo momento e aggiungere le astrazioni aggiuntive se ne hai bisogno.

Penso che il problema fondamentale del tuo design sia che stai introducendo un nuovo tipo per ciascun modo di combinare i tuoi tipi di dati e soprattutto che rendi privata la struttura delle tue canzoni.

Immagina se stavo creando una libreria matematica, e avevo un tipo di "interi semplici" che sono solo letterali. E poi ho creato un separato tipo "interi aggiunti" per la somma di due interi semplici. E poi un tipo separato "numeri interi moltiplicati" per la moltiplicazione di due interi aggiunti. Ora, oltre a ciò, mantengo il pattern di bit che rende i numeri un segreto, quindi le uniche operazioni matematiche che saranno mai possibili sono quelle codificate nella mia libreria. Questo è quello che stai facendo.

Penso che potresti ottenere ciò che desideri rappresentando tutto come sequenze di accordi (preferibilmente immutabili) e rendendo quella sequenza pubblica . Puoi ri-implementare i tuoi generatori come semplici vecchie funzioni che duplicano, trasformano e incollano sequenze di accordi in particolari modi. In alternativa, le canzoni potrebbero essere stream di accordi, prodotte pigramente, che ti consentirebbero di generare canzoni infinite. O il design finisce molto più semplice e non c'è limite al numero di modi in cui è possibile comporre canzoni.

Prova a pensare a un grosso problema è sempre complicato, provo a concentrarti su una piccola parte, dici:

"SimpleMelody è composto da una serie di note: l'esecuzione di una riproduzione suona queste note una dopo l'altra .. StructuredMelody è composta da una serie di oggetti Melody, che invocano la riproduzione su Melodyies una dopo l'altra."

Vedo una mancanza di astrazione che genera la duplicazione, puoi pensare al problema in altro modo:

"La melodia è composta da una serie di oggetti giocabili. Invocare la riproduzione suona questi oggetti giocabili uno dopo l'altro"

Non hai bisogno di due classi diverse (melody e structuredMelody) per fare la stessa cosa. I generatori di melodia possono occuparsi di comporre la melodia con i "pezzi" giusti.

Probabilmente con lo stesso approccio puoi semplificare molto il resto del sistema.

Progetta mentre parli

Come già sottolineato, hai alcune cose puzzolenti in corso.

Dici:

StructuredTune owns two Tune instances. It's own play plays the two Tunes one after the other according to a pattern (currently only A-B-A-B)

La tua implementazione comunque:

StructuredTune is a Tune that owns 2 tunes.

eh? Wow, euhmm. eh?

Ora leggi di nuovo quanto sopra, ma cambia Tune with Melody.

eh?

Ora leggerlo un'altra volta, ma ora sostituisci Tune with Progression

eh?

WTFsPM

Conosciuto anche come WTF / minuto

E inizia con il quadro generale. A StructuredMelody conosce Melodies dove SimpleMelody conosce solo Notes.

Questo sarebbe ok, perché ora StructuredMelody potrebbe contenere più StructuredMelodies che hanno più StructuredMelodies. Cool!

Poi abbiamo anche un StructuredTune che contiene più brani che contengono ciascuno una melodia strutturata che contiene più StructuredMelodies che ...

eh?

Quindi StructuredTune è in realtà un metodo StructuredMelody. Solo invece di avere Melodie direttamente contiene brani che contengono una melodia. Wow! Ma ora hai anche la possibilità di aggiungere Progressions (anche conosciuto come Chords?).

Ora leggi di nuovo lo stesso, ma con un'ulteriore ripetizione di quelle Progressioni in esse.

Mi sono perso

No davvero, sono un musicista e (dovrei) sapere molto sulla musica. Ma mi hai perso, molto tempo fa.

Il motivo sbagliato per il motivo sbagliato

questo è il tuo problema :) Hai trovato uno schema interessante e lo stai usando ovunque. Non è bello!

Side note: been there done that. I abused the SingletonPattern for over a year, just because I liked it.

Il modo migliore

better because nothing is 'best'. We just have better solutions.

Per prima cosa devi scegliere un approccio. Si potrebbe andare per l'approccio pitch + note dove una nota consiste di un pitch e una durata. Oppure dimentica l'altezza e usa solo una nota e gestisci la durata da qualche altra parte.

Dovresti anche decidere su come stai usando gli accordi. È un accordo un tono di radice con una qualità di accordo? o alcune scene stipate insieme. O note?
Un accordo potrebbe avere 3 note, ognuna con una durata diversa. schifo: (

Quindi la nostra progressione. È una roba giocabile stipata insieme? O in realtà intendiamo una progressione di accordi che come una radice e una progressione (cioè I-IV-V)? O dovremmo chiamarlo Harmony che ha una rootTonality con una progressione? perso di nuovo.

A seconda di cosa stai costruendo, potrebbero arrivare diverse esigenze. Potresti anche scegliere un approccio in cui accordi e progressioni non esistono.

Solo note che hanno un tono e una durata.

Un motivo quindi è una raccolta di note e qualche tipo di conoscenza su quando iniziare quale nota. Non abbiamo bisogno di accordi qui;)

O anche più semplice. Potremmo definire una nota con un tono, un'ora di inizio e uno stop. Stupido semplice.

I tuoi nomi sono sbagliati

I nomi sono importanti. Sono la cosa più importante nel tuo software. E definirli è altrettanto importante.

Personalmente userò il seguente approccio:

StartStoppables:

• A Pitch
• A Chord ha più Pitch

La nostra interfaccia StartStoppable:

interface StartStoppable
{
    public function start();
    public function stop();
}

The Playables:

• Un Note ha un Pitch e un Duration
• A Harmony ha Chord e a Duration

La nostra interfaccia riproducibile:

interface Playable
{
    public function play();
}

Le cose relative alla progressione:

• Un Progression definisce una strategia di progressione (cioè V-IV-I-I, possono essere usate altre notazioni di progressione)
• A ChordProgression ha più Harmony ed è anche Playable (!)

Quindi aggiungiamo alcune fabbriche per aiutarci nella creazione dei nostri oggetti.

• a PitchFactory puoi creare un Pitch dato un valore e la notazione usata (cioè Neo-Latino)
• a ChordFactory può creare un Chord da un dato Pitch e qualità dell'accordo.
• a HarmonyFactory può creare un Harmony da un dato Pitch e HarmonySet (cioè MajorTriad).
• a HarmonyProgressionFactory può creare una HarmonyProgression da un dato Chord e a Progression

Il nostro PitchFactory sarebbe simile a questo:

class PitchFactory
{
    const ENGLISH = 0; //default notation
    const NEOLATINN = 1;

    public static function create(String $value, int $notation=0) {...}
}

E poi usato come:

$pitch = PitchFactory::crate('DO', PitchFactory::NEOLATIN);
//or
$pitch = PitchFactory::create('C');;

Il nostro ChordFactory potrebbe essere usato come segue:

$chord = ChordFactory::create($pitch, ChordFactory::MAJOR); //Major chord

Come notato in altre risposte, questi metodi di creazione potrebbero essere spostati negli oggetti stessi per creare un modo più piacevole di codifica:

$pitch = Pitch::create('DO', PITCH::NEOLATIN);
$chord = Chord::create($pitch, Chord::MAJOR);

Possiamo quindi iniziare a utilizzare le nostre note e le nostre armonie:

$length = 1; //whatever this means

$note = new Note($pitch, $length);

$harmony = new Harmony($chord, $length);

//or maybe a minor triad with root set to $pitch
$harmony = Harmony::create($pitch, $length, Harmony::MinorTriad);

E infine il nostro HarmonyProgression :

$progression = new Progression('V-IV-I-I');
$harmonyProgression = HarmonyProgressionFactory::create($chord, $progression);

In seguito le annotazioni potrebbero essere prese in considerazione in una propria classe. Ciò ti darà la flessibilità di poter usare le notazioni Tabular, Roman, ecc. Indipendenti dal resto della tua applicazione.

Sembra che tu abbia visto (o reinventato) il "pattern composito" da qualche parte, e ora lo stai lanciando a tutti i tuoi oggetti principali (melodie, melodie, progressione), senza sapere se è realmente necessario o meno ("nel caso"). Questo approccio ha un alto rischio di sovradimensionamento delle cose. Il design attuale consentirà di modellare gerarchie ad albero di melodie, melodie e progressioni, per le quali la struttura ad albero di ciascuna di esse è completamente indipendente l'una dall'altra. Ne hai davvero bisogno per la tua applicazione? È necessario creare un semplice sintetizzatore da un

• gerarchia di melodie ad albero e un secondo
• gerarchia gerarchica indipendente delle progressioni?

O combinare una melodia strutturata composta da 3 semplici melodie con un'altra melodia composta da 2 melodie semplici si traduce in una melodia strutturata di 5 melodie, dove il raggruppamento originale non ha più importanza in seguito? Se è il caso, proverei a iniziare solo con una classe Tune, tenendo un elenco di melodie e un elenco di progressioni.

E perché hai bisogno di un "albero di melodie" completo?

E anche se sei convinto di aver bisogno di tutte quelle gerarchie, non è sicuramente chiaro il motivo per cui la gerarchia del generatore deve seguire il modello composito. Immagino che avrai bisogno al massimo di un generatore di oggetti, quindi perché non creare, ad esempio, una classe MelodyGenerator, con una sola istanza di oggetto, che verrà utilizzata per produrre molti oggetti melodici (che saranno di tipo SimpleMelody o StructuredMelody ). Quindi, se hai bisogno di un metodo "generateSimpleMelody" e un metodo "generateStructuredMelody", rendi due metodi della classe del generatore, non introdurre una propria classe ogni volta che devi semplicemente aggiungere un altro metodo. Un albero completo come la gerarchia degli oggetti MelodyGenerator sembra IMHO essere piuttosto inutile.

Tuttavia, "la forma segue la funzione", quindi proverei effettivamente a implementare una parte della tua applicazione, iniziando dal più semplice modello di oggetto che puoi immaginare (il suggerimento di Robert Harvey suona bene, una Melodia con una serie di note forse sufficiente per la prima versione della tua applicazione) ed estendi il tuo modello ogni volta che la tua applicazione ha bisogno di ottenere una funzione che non puoi risolvere con semplici classi di raccolta.