È una tecnica scadente avere funzioni all'interno di una classe che dipendono l'una dall'altra in modo "a cascata"?

Ciò che hai sembra buono.

Hai diverse azioni setEmailAddresses , getEmailAddresses ciascuna delle quali comprende una serie di passaggi. Alcuni di questi passaggi sono comuni a più azioni, quindi è perfettamente logico includerli nei propri metodi.

La parte restante dell'azione potrebbe essere codificata nel metodo di livello superiore, ma averlo nel proprio "metodo secondario" non è un problema - in effetti si potrebbe dire che è una pratica migliore in quanto si mantiene responsabile ciascun metodo solo una cosa.

Per rispondere alla tua domanda, no, non è affatto una cattiva pratica dividere gli interni di una classe in più metodi come hai fatto qui. Come hanno detto le altre risposte e commenti, è in realtà una buona pratica.

Sei sulla strada giusta, ma ho visto un paio di cose che penso abbiano bisogno di attenzione. Dall'esempio fornito all'esterno della definizione della classe, sembra che l'uso principale di oggetti derivati da questa classe sia (approssimativamente) per estrarre gli indirizzi email da una query PubMed. Hai diviso le attività generali necessarie per farlo in tutta la classe in modo appropriato; tuttavia, molti di questi compiti si limitano a impostare alcune informazioni immutabili. Fai questo tipo di metodi, ad es. setSearchParameters (), ha davvero bisogno di essere accessibile al di fuori della classe? Direi che no, non lo fanno, perché la maggior parte di questi metodi non fa altro che ottenere o impostare variabili dei membri privati.

Espongo l'intero problema di incapsulamento perché un modo in cui potresti migliorare il tuo codice sarebbe quello di eliminare alcune delle variabili dei membri private a meno che tu non ne abbia assolutamente bisogno. Non conosco i requisiti del tuo progetto, ma mi sembra che ogni query PubMed sia rappresentata dal suo stesso oggetto. In tal caso, è necessario rendere, ad esempio, la variabile $ searchParameters un membro della classe? Perché non fare semplicemente qualcosa del tipo:

public function setSearchParameters() {
    return array(
        'db'         => 'pubmed',
        'term'       => $this->query,
        'retmode'    => 'xml',
        'retstart'   => '0',
        'retmax'     => '1000',
        'usehistory' => 'y'
    );
}

E poi, per il prossimo metodo nella sequenza:

public function setSearchURL() {
    $baseSearchURL = 'http://eutils.ncbi.nlm.nih.gov/entrez/eutils /esearch.fcgi?';
    return $baseSearchURL . http_build_query($this->setSearchParameters());

}

Perché farlo? Cerca di pensare a cosa sia realmente una variabile membro: è una rappresentazione dello stato dell'oggetto in questione. Il membro $ query ha senso; rappresenta lo stato di base dell'oggetto PubMedQuery. Funzionando all'indietro dal metodo di livello superiore, getEmailAddresses (), tutto nell'oggetto dipende essenzialmente dal membro $ query. Almeno nel codice che hai fornito, gli altri metodi funzionano semplicemente su questa variabile membro in successione. (Probabilmente avrei impostato la variabile $ baseSearchURL come costante anziché come variabile, ma questo va oltre lo scopo di questa risposta).

Rimuovendo la variabile membro $ searchParameters e strutturando il codice più come mostrato sopra, si elimina anche la necessità del metodo getter, che riduce l'accoppiamento. Se per qualche motivo un altro oggetto ha bisogno della variabile $ searchParameters non membro prodotta dal metodo setSearchParameters, è possibile mantenere pubblico quel metodo. Se sei sicuro che nessun altro oggetto ne avrà bisogno, rendilo privato o protetto. Ancora una volta, non conosco le tue esigenze, ma sospetto che nulla oltre al tuo metodo di alto livello debba essere pubblico. Per ridurre ulteriormente l'accoppiamento, se rendi pubblico solo il tuo metodo di primo livello, puoi definirlo come un metodo di interfaccia. In questo modo, gli oggetti che dipendono dall'oggetto PubMedQuery possono invece dipendere dall'interfaccia.

In genere tendo a provare a ripensare il design di una classe se finisco con più di alcune variabili membro. Se risulta che ho davvero bisogno di un oggetto per tenere traccia di tanti stati, comincio a pensare che forse potrei voler rifattorizzare parte del codice di quell'oggetto in altri oggetti. Un oggetto dovrebbe avere solo una responsabilità. Sono sicuro che i casi esistano, ma personalmente non ho mai dovuto creare un oggetto che fosse necessario tenere traccia di più di quattro o cinque variabili membro al massimo per soddisfare tale responsabilità.

Nel complesso, sei decisamente sulla strada giusta. La suddivisione della responsabilità generale di un oggetto in più metodi che dipendono l'uno dall'altro all'interno di una classe è sicuramente una buona pratica. Quello che ho discusso qui è semplicemente portare le cose al livello successivo.