Para responder à sua pergunta, não, não é nada ruim dividir as partes internas de uma classe por vários métodos, como você fez aqui. Como as outras respostas e comentários disseram, é realmente uma boa prática.
Você está no caminho certo, mas eu vi algumas coisas que acho que precisam de atenção. Do exemplo que você dá fora da definição de classe, parece que o uso primário de objetos derivados dessa classe é (aproximadamente) extrair endereços de email de uma consulta PubMed. Você dividiu as tarefas gerais necessárias para fazer isso em toda a classe de forma adequada; no entanto, muitas dessas tarefas simplesmente definem algumas informações imutáveis. Estes tipos de métodos, por ex. setSearchParameters (), realmente precisa estar acessível fora da classe? Eu diria que não, eles não, porque a maioria desses métodos não faz nada além de obter ou definir variáveis de membros privados.
Eu apresento todo o problema de encapsulamento porque uma maneira de melhorar seu código seria descartar algumas das variáveis de membros privados, a menos que você realmente precise delas. Eu não sei os requisitos do seu projeto, mas parece-me que cada consulta PubMed sendo feita é representada por seu próprio objeto. Em caso afirmativo, é necessário tornar, por exemplo, a variável $ searchParameters um membro da classe? Por que não simplesmente fazer algo como:
public function setSearchParameters() {
return array(
'db' => 'pubmed',
'term' => $this->query,
'retmode' => 'xml',
'retstart' => '0',
'retmax' => '1000',
'usehistory' => 'y'
);
}
E depois, para o próximo método na sequência:
public function setSearchURL() {
$baseSearchURL = 'http://eutils.ncbi.nlm.nih.gov/entrez/eutils /esearch.fcgi?';
return $baseSearchURL . http_build_query($this->setSearchParameters());
}
Por que isso? Tente pensar sobre o que uma variável membro realmente é - é uma representação do estado do objeto em questão. O membro $ query faz sentido; representa o estado básico do objeto PubMedQuery. Trabalhando de trás para frente a partir do método de nível superior, getEmailAddresses (), tudo no objeto depende essencialmente do membro $ query. Pelo menos no código que você forneceu, os outros métodos simplesmente operam nessa variável de membro em sucessão. (Eu provavelmente definiria a variável $ baseSearchURL como uma constante em vez de uma variável, mas isso está além do escopo desta resposta).
Remover a variável de membro $ searchParameters e estruturar seu código mais como o mostrado acima também elimina a necessidade do método getter, que reduz o acoplamento. Se por algum motivo outro objeto precisar da variável $ searchParameters de não membro produzida pelo método setSearchParameters, você poderá manter esse método público. Se tiver certeza de que nenhum outro objeto precisará, torne-o privado ou protegido. Novamente, não conheço seus requisitos, mas suspeito que nada além do seu método de nível superior realmente precise ser público. Para reduzir ainda mais o acoplamento, se você tornar público apenas seu método de nível superior, você poderá defini-lo como um método de interface. Dessa forma, objetos que dependem do seu objeto PubMedQuery podem depender da interface.
Eu geralmente tento repensar o design de uma classe se eu tiver mais do que algumas variáveis de membros. Se eu realmente precisar de um objeto para acompanhar esses muitos estados, começo a pensar que talvez eu queira refatorar parte do código desse objeto em outros objetos. Um objeto deve ter apenas uma responsabilidade. Tenho certeza de que existem casos, mas eu pessoalmente nunca tive que fazer um objeto que precisasse acompanhar mais de quatro ou cinco variáveis de membros, no máximo, para cumprir essa responsabilidade.
No geral, você está definitivamente no caminho certo. Subdividir a responsabilidade geral de um objeto em vários métodos que dependem uns dos outros dentro de uma classe é definitivamente uma boa prática. O que eu discuti aqui é simplesmente levar as coisas para o próximo nível.