Scraping PubMed query results: follow-up

Question

I recently posted a question looking for feedback on a script I wrote (see: Scraping PubMed query results)

Since then I have re-written it into a class, which I have posted below. How does it look? How can I improve upon it?

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._%+-]+@[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());

Answer 1

The first thing I might do is get rid of the email regex matching. PHP has this awesome functionality called filter_var that will take care of a lot of cool things for you, like validating a string as an email address. This is a better solution because you're less likely to get back false positives where the email is valid, but doesn't match the regex.

Next, I'm not sure I like the search/fetch parameters being set at various places throughout the code. This feels like something that should be done in the constructor; setup all the data you need to complete the responsibility of the class in the constructor. If you ever add a new method or make some changes you always have to keep in the back of your mind "Have I set $x yet?". Well, do it in __construct() and you'll know you have.

Finally, I noticed you were calling some get* functions but not assigning the return value to any variable. If you don't use the return value you don't need to use the get* function and if you don't use the get* function you don't need it in the class.

Overall I like the API, the method names are fairly self explanatory. I'm not sure that they should all be public as your use case only has one method actually being used from calling code. In addition, I might change the properties from private to protected for extensibility but that is certainly not mandatory and is as much a preference as anything else. I would also prefer to see some kind of documentation, even if it is just the responsibility of the class and your though processes behind why you solved the problem this way.