É uma técnica pobre ter funções dentro de uma classe que dependem umas das outras de uma forma tipo “cascata”?

5

Eu sou relativamente novo em PHP e OOP. Dito isto, eu queria saber como funções independentes deveriam estar dentro de uma classe. Eu sei que cada função deve ser responsável por fazer apenas uma coisa. No entanto, como você pode ver na minha classe abaixo, ocasionalmente me vejo criando funções que constroem umas sobre as outras de uma forma tipo "cascata".

Esta é uma boa prática? Existe uma maneira diferente de estruturar essa classe / esses tipos de funções? Eu quero ter certeza de que estou desenvolvendo bons hábitos.

Agradecemos antecipadamente por sua ajuda.

class PubMedQuery {

private $query;
private $searchParameters;
private $searchURL;
private $fetchParameters;
private $fetchURL;
private $searchResults;
private $fetchResults;
private $matches;
private $matchRegex;
private $emailAddresses;

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

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

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

public function setFetchParameters() {
    $this->fetchParameters = array(
        'db'        => 'pubmed',
        'retmax'    => '1000',
        'query_key' => (string) $this->searchResults->QueryKey,
        'WebEnv'    => (string) $this->searchResults->WebEnv
    );
}

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

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

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

public function setFetchURL() {
    $this->baseFetchURL = 'http://eutils.ncbi.nlm.nih.gov/entrez/eutils/efetch.fcgi?';
    $this->fetchURL = $this->baseFetchURL . http_build_query($this->getFetchParameters());
}

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

public function setSearchResults() {
    $this->setSearchParameters();  
    $this->setSearchURL();
    $this->searchResults = simplexml_load_file($this->getSearchURL());
}

public function getSearchResults() {
    $this->setFetchParameters();
    $this->setFetchURL();
    return file_get_contents($this->getFetchURL()); 
}

public function setEmailAddresses() {
    $this->matches = array();
    $this->matchRegex = '/[A-Za-z0-9._%+-][email protected][A-Za-z0-9.-]+\.[A-Za-z]{2,4}/'; 
    preg_match_all($this->matchRegex, $this->getSearchResults(), $this->matches);
    $this->emailAddresses = array_unique(array_values($this->matches[0]));
}

public function getEmailAddresses() {
    $this->setSearchResults();
    $this->getSearchResults();
    $this->setEmailAddresses();
    return $this->emailAddresses;
}
}

//Example using search term "psoriasis"
$query  = new PubMedQuery('psoriasis');
echo implode('<br />', $query->getEmailAddresses());
    
por LookingForHelpWithArray 21.10.2011 / 16:40
fonte

2 respostas

6

O que você tem parece ser bom.

Você tem várias ações setEmailAddresses , getEmailAddresses , cada uma delas composta de uma série de etapas. Algumas dessas etapas são comuns em várias ações, por isso faz todo o sentido incluí-las em seus próprios métodos.

A parte restante da ação pode ser codificada no método de nível superior, mas tê-la em seu próprio "sub-método" não é um problema - na verdade, pode-se dizer que é uma prática melhor, pois você está mantendo cada método responsável apenas uma coisa.

    
por 21.10.2011 / 16:47
fonte
2

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.

    
por 23.10.2011 / 05:02
fonte