patternphpMinor
Checking for existence of URL parameters
Viewed 0 times
existencecheckingforparametersurl
Problem
I'm busy rewriting some from my spaghetti code into OOP classes. I'm still learning OOP and very new to the concept. From help from this answer from @EliasVanOotegemto on a previous question I've asked I have written the following class which sets up conditionals according to 4 values passed to it by a user and checking these values against URL parameters whether they exist or not
Here is the class with its interface:
ConditionsRefInterface.php
ConditionsRef.php
```
namespace PG\Referrer\Single\Post;
class ConditionsRefer implements ConditionsReferInterface {
/**
* @var $authorReferrer = null
*/
protected $isAuthorReferrer = null;
/**
* @var $dateReferrer = null
*/
protected $isDateReferrer = null;
/**
* @var $searchReferrer = null
*/
protected $isSearchReferrer = null;
/**
* @var $taxReferrer = null
*/
protected $isTaxReferrer = null;
/**
* @param array $values = null;
*
* Accepted parameters that can be passed to $values.
* @param (string) authorReferrer
* @param (
Here is the class with its interface:
ConditionsRefInterface.php
namespace PG\Referrer\Single\Post;
interface ConditionsReferInterface
{
/**
* @param (string)$authorReferrer
* @return $this
*/
public function setAuthorReferrer($isAuthorReferrer);
/**
* @param (string)$dateReferrer
* @return $this
*/
public function setDateReferrer($isDateReferrer);
/**
* @param (string)$searchReferrer
* @return $this
*/
public function setSearchReferrer($isSearchReferrer);
/**
* @param (string)$taxReferrer
* @return $this
*/
public function setTaxReferrer($isTaxReferrer);
/**
* @return (bool)
*/
public function isAuthorReferrer();
/**
* @return (bool)
*/
public function isDateReferrer();
/**
* @return (bool)
*/
public function isSearchReferrer();
/**
* @return (bool)
*/
public function isTaxReferrer();
}ConditionsRef.php
```
namespace PG\Referrer\Single\Post;
class ConditionsRefer implements ConditionsReferInterface {
/**
* @var $authorReferrer = null
*/
protected $isAuthorReferrer = null;
/**
* @var $dateReferrer = null
*/
protected $isDateReferrer = null;
/**
* @var $searchReferrer = null
*/
protected $isSearchReferrer = null;
/**
* @var $taxReferrer = null
*/
protected $isTaxReferrer = null;
/**
* @param array $values = null;
*
* Accepted parameters that can be passed to $values.
* @param (string) authorReferrer
* @param (
Solution
GET and setters
One of the points of @EliasVanOotegemto was that your class expects the
That is why @EliasVanOotegemto suggested to pass the value contained in GET instead of passing the index at which the value is stored in GET. But through your new design, GET is actually used in two classes instead of one, which couples your classes together, and makes both hard to reuse.
Your setters should actually look really simple:
And then used eg like this:
Naming
Comments
Misc
One of the points of @EliasVanOotegemto was that your class expects the
$_GET variable to be set, which is not ideal because it makes your class very static and hard to reuse. What if you at some point do not store this data in GET, but in POST? Or in a session? You cannot use your class in that case.That is why @EliasVanOotegemto suggested to pass the value contained in GET instead of passing the index at which the value is stored in GET. But through your new design, GET is actually used in two classes instead of one, which couples your classes together, and makes both hard to reuse.
Your setters should actually look really simple:
public function setDateReferrer($isDateReferrer)
$this->isDateReferrer = $isDateReferrer;
return $this;
}And then used eg like this:
conditionsRefer->setDateReferrer($_GET['dateReferer']);Naming
- class names should ideally be the same as the file in which they are stored (possibly with the keyword
classadded.ConditionsRef.phpisn't a very good name (What's aRef?),ConditionsReferer.php,ConditionsReferer.class.php,conditionsReferer.class.phpwould all be better (just choose one naming theme, and then stick with it).
- be consistent: either it's
refererorreferrer.
isVariableNameusually implies a boolean value. YourisDateReferreretc do not seem to be boolean values though.
Comments
- your auto-generated comments seem wrong (eg
@param (string)$authorReferrervs$isAuthorReferrer, orprotected $isAuthorReferrervs@var $authorReferrer).
- sometimes, auto-generated comments might be fine, but I would prefer some additional information. For example, what is a
taxReferrer? What values are$values?
- class level comments would be great. What is a
ConditionsRefer?
Misc
- I'm not sure why you use
filter_varin your code. If you want to protect against XSS attacks, I would clean the data when echoing it to the enduser, not at any other point (otherwise it becomes a guessing game when echoing data: did I already clean it?).
- I don't like using
switch(true)instead ofif-else, it just looks really wrong.
- do you actually expect a different implementations of
ConditionsReferInterfacethanConditionsRefer? It's hard for me to imagine one. If there is only one possible implementation of an interface, the interface isn't really needed.
Code Snippets
public function setDateReferrer($isDateReferrer)
$this->isDateReferrer = $isDateReferrer;
return $this;
}conditionsRefer->setDateReferrer($_GET['dateReferer']);Context
StackExchange Code Review Q#80601, answer score: 5
Revisions (0)
No revisions yet.