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
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 (string) dateReferrer
* @param (string) searchReferrer
* @param (string) taxReferrer
*
*/
public function __construct(array $values = null)
{
if(isset($values['authorReferrer']))
$this->setAuthorReferrer($values['authorReferrer']);
if(isset($values['dateReferrer']))
$this->setDateReferrer($values['dateReferrer']);
if(isset($values['searchReferrer']))
$this->setSearchReferrer($values['searchReferrer']);
if(isset($values['taxReferrer']))
$this->setTaxReferrer($values['taxReferrer']);
}
/**
* @param (string)$authorReferrer
* @return $this
*/
public function setAuthorReferrer($isAuthorReferrer)
{
$isAuthorReferrer = filter_var($isAuthorReferrer, FILTER_SANITIZE_STRING);
$isAuthorReferrer = isset($_GET[$isAuthorReferrer]);
$this->isAuthorReferrer = $isAuthorReferrer;
return $this;
}
/**
* @param (string)$dateReferrer
* @return $this
*/
public function setDateReferrer($isDateReferrer)
{
$isDateReferrer = filter_var($isDateReferrer, FILTER_SANITIZE_STRING);
$isDateReferrer = isset($_GET[$isDateReferrer]);
$this->isDateReferrer = $isDateReferrer;
return $this;
}
/**
* @param (string)$searchReferrer
* @return $this
*/
public function setSearchReferrer($isSearchReferrer)
{
$isSearchReferrer = filter_var($isSearchReferrer, FILTER_SANITIZE_STRING);
$isSearchReferrer = isset($_GET[$isSearchReferrer]);
$this->isSearchReferrer = $isSearchReferrer;
return $this;
}
/**
* @param (string)$taxReferrer
* @return $this
*/
public function setTaxReferrer($isTaxReferrer)
{
$isTaxReferrer = filter_var($isTaxReferrer, FILTER_SANITIZE_STRING);
$isTaxReferrer = isset($_GET[$isTaxReferrer]);
$this->isTaxReferrer = $isTaxReferrer;
return $this;
}
/**
* @return (bool)
*/
public function isAuthorReferrer()
{
return $this->isAuthorReferrer;
}
/**
* @return (bool)
*/
public function isDateReferrer()
{
return $this->isDateReferrer;
}
/**
* @return (bool)
*/
public function isSearchReferrer()
{
return $this->isSearchReferrer;
}
/**
* @return (bool)
*/
public function isTaxReferrer()
{
return $this->isTaxReferrer;
}
}
This works.
I have written another class that will use the conditions set by the class above. This is how it works
User sets the arguments in this new class (
QueryArgumentsRef.php
) which is passed to theConditionsRef
class through the constructo methodpublic function __construct(ConditionsReferInterface $conditionalReferrer, array $values = null)
The
get*
methods from theConditionsRef
class is then used to get the conditional values through$this->conditionalReferrer->isAuthorReferrer()
for instance$referArgs
returns values according to the user set arguments
Here is the interface and class:
QueryArgumentsRefInterface.php
namespace PG\Referrer\Single\Post;
interface QueryArgumentsReferInterface
{
/**
* @return (array) arguments
*/
public function getReferrerArgs();
}
QueryArgumentsRef.php
namespace PG\Referrer\Single\Post;
class QueryArgumentsRefer implements QueryArgumentsReferInterface
{
/**
* @var $conditionalReferrer = null
*/
protected $conditionalReferrer = null;
/**
* @var $values = null
*/
protected $values = null;
/**
* @var $referArgs = null
*/
protected $referArgs = null;
/**
* @param $conditionalReferrer
* @param array $values = null;
*
* Accepted parameters that can be passed to $values.
* @param (string) authorReferrer
* @param (string) dateReferrer
* @param (string) searchReferrer
* @param (string) taxReferrer
*
*/
public function __construct(ConditionsReferInterface $conditionalReferrer, array $values = null)
{
$this->conditionalReferrer = $conditionalReferrer;
$this->values = $values;
if(isset($values['authorReferrer']))
$this->conditionalReferrer->setAuthorReferrer($values['authorReferrer']);
if(isset($values['dateReferrer']))
$this->conditionalReferrer->setDateReferrer($values['dateReferrer']);
if(isset($values['searchReferrer']))
$this->conditionalReferrer->setSearchReferrer($values['searchReferrer']);
if(isset($values['taxReferrer']))
$this->conditionalReferrer->setTaxReferrer($values['taxReferrer']);
$this->referrerArgs();
}
public function referrerArgs()
{
switch (true) {
case ($this->conditionalReferrer->isAuthorReferrer() && isset($this->values['authorReferrer'])):
/**
* If the referrer came from an author archive page
*/
$author = $this->values['authorReferrer'];
$author = filter_var($author, FILTER_SANITIZE_STRING);
$referArgs = ['author' => (int) $_GET[$author]];
break;
case ($this->conditionalReferrer->isDateReferrer() && isset($this->values['dateReferrer'])):
/**
* If the referrer came from a date archive page
*/
$date = $this->values['dateReferrer'];
//$referArgs = (STILL TO WORKOUT);
break;
case ($this->conditionalReferrer->isSearchReferrer() && isset($this->values['searchReferrer'])):
/**
* If the referrer came from an search page
*/
$search = $this->values['searchReferrer'];
$search = filter_var($search, FILTER_SANITIZE_STRING);
$search_terms = $_GET[$search];
$referArgs = ['s' => (string) $search_terms];
break;
case ($this->conditionalReferrer->isTaxReferrer() && isset($this->values['taxReferrer'])):
/**
* If the referrer came from a taxonomy archive page
*/
$tax = $this->values['taxReferrer'];
$tax = filter_var($tax, FILTER_SANITIZE_STRING);
$terms = $_GET[$tax];
$terms = explode( ' ', $terms );
$taxonomy_objects = get_taxonomy( $terms[0] );
$hierarchical = $taxonomy_objects->hierarchical;
$referArgs = ['tax_query' => [
[
'taxonomy' => (string) $terms[0],
'terms' => (int) $terms[1],
'include_children' => false
]
]];
break;
default:
$referArgs = null;
}
$this->referArgs = (array) $referArgs;
return $this;
}
/**
* @return (array) query arguments according to referrer
*/
public function getReferrerArgs()
{
return $this->referArgs;
}
}
I can now use it at follows:
$a = new QueryArgumentsRefer(new ConditionsRefer(), ['authorReferrer' => 'aq', 'taxReferrer' => 'tq']);
var_dump($a->getReferrerArgs());
where var_dump($a->getReferrerArgs());
produces the following output if I'm on a page where the query variable tq
is set in the URL and the user have set the taxReferrer
arguments
array(1) {
["tax_query"]=>
array(1) {
[0]=>
array(3) {
["taxonomy"]=>
string(8) "category"
["terms"]=>
string(1) "1"
["include_children"]=>
bool(false)
}
}
}
This will later be used to set up a new instance of WP_Query
in another class.
Is the above code correct, or is there flaws that needs to be addressed? I welcome any constructive feedback, whether good or bad.
2 Answers 2
GET and setters
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
class
added.ConditionsRef.php
isn't a very good name (What's aRef
?),ConditionsReferer.php
,ConditionsReferer.class.php
,conditionsReferer.class.php
would all be better (just choose one naming theme, and then stick with it). - be consistent: either it's
referer
orreferrer
. isVariableName
usually implies a boolean value. YourisDateReferrer
etc do not seem to be boolean values though.
Comments
- your auto-generated comments seem wrong (eg
@param (string)$authorReferrer
vs$isAuthorReferrer
, orprotected $isAuthorReferrer
vs@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_var
in 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
ConditionsReferInterface
thanConditionsRefer
? It's hard for me to imagine one. If there is only one possible implementation of an interface, the interface isn't really needed.
-
\$\begingroup\$ Thanks for your input. The naming and comments for some reason got haywire and mixed, great spot. Already fixed that :-). On the
switch
, it is really more personal choice. What goes for one does not go for another \$\endgroup\$Pieter Goosen– Pieter Goosen2015年02月17日 18:35:25 +00:00Commented Feb 17, 2015 at 18:35 -
1\$\begingroup\$ I found a really useful post on the programmer SE site regarding how to use superglobals like
$_GET
in OOP. Based on your recommendation and on recommendation from the answers on my previous question not to use superglobals in the manner in which I did, I'm going to create aRequest
class to handle the superglobals and then pass that to my other classes where needed. This will make my classes more testable. Note, the accepted answer there needs a serious rewrite :-) \$\endgroup\$Pieter Goosen– Pieter Goosen2015年02月18日 05:57:42 +00:00Commented Feb 18, 2015 at 5:57
Thanks for all the help. I ended up rewriting everything into one class. My main reason for this is that breaking this up into 3 three classes, it makes 2 of the classes a real mess through interface injection and quite hard to unit test. I think that the bulkiness of the class makes it quite difficult to maintain as separate smaller classes as we are working with 4 variables
At the end of the day, this class still deals with one concern in the complete system to follow, that is, testing 4 given variables against the returned array from a super global and returning the result as array in the form of query arguments and query variables.
It is not as clean and true as I would want it to be, and I'm sure the new class can be improved, but that is something for the near future.
CHANGES
Removed the hardcoded
$_GET
variable as suggested by the accepted answer and linked answerCorrected the naming conventions used
Better commenting in doc blocks
Used
htmlspecialchars()
to guard against XSS only when the values from the URL (super global) is returned as query arguments and new query variables that is set in the URLChanged the
switch()
to a normalif/else
statements.Removed the methods from the constructor
Here is my new class and interface
RequestReferrerHandlerInterface.php
<?php
namespace PG\Single\Post\Navigation;
interface RequestReferrerHandlerInterface
{
/**
* Setter setSuperGlobalVar
* @param $superGlobalVar
* @return $this
*/
public function setSuperGlobalVar($superGlobalVar);
/**
* Returns an array of super global variables
* @return (array) $this->superGlobalVar
*/
public function getSuperGlobalVar();
/**
* Setter setAuthorReferrer
* @param $authorReferrer
* @return $this
*/
public function setAuthorReferrer($authorReferrer);
/**
* Returns the value of the $authorReferrer property
* @return (array) $this->authorReferrer
*/
public function getAuthorReferrer();
/**
* Setter setDateReferrer
* @param $dateReferrer
* @return $this
*/
public function setDateReferrer($dateReferrer);
/**
* Returns the value of the $dateReferrer property
* @return (array) $this->dateReferrer
*/
public function getDateReferrer();
/**
* Setter setSearchReferrer
* @param $searchReferrer
* @return $this
*/
public function setSearchReferrer($searchReferrer);
/**
* Returns the value of the $searchReferrer property
* @return (array) $this->searchReferrer
*/
public function getSearchReferrer();
/**
* Setter setTaxReferrer
* @param $taxReferrer
* @return $this
*/
public function setTaxReferrer($taxReferrer);
/**
* Returns the value of the $taxReferrer property
* @return (array) $this->taxReferrer
*/
public function getTaxReferrer();
/**
* Test $authorReferrer against $superGlobalVar
*
* @return (bool) true on success or false on failure
*/
public function isAuthorReferrer();
/**
* Test $authorReferrer against $superGlobalVar
*
* @return (bool) true on success or false on failure
*/
public function isDateReferrer();
/**
* Test $authorReferrer against $superGlobalVar
*
* @return (bool) true on success or false on failure
*/
public function isSearchReferrer();
/**
* Test $authorReferrer against $superGlobalVar
*
* @return (bool) true on success or false on failure
*/
public function isTaxReferrer();
/**
* Conditional which check if the current post is a referred post
*
* @return (bool) true on success or false on failure
*/
public function isReferredPost();
/**
* Sets the values retrieves from the super global to an array
* Will be passed as query arguments in a new instance of WP_Query
*
* @return (array) $queryArgs
*/
public function referrerQueryArguments();
/**
* Gets the values from the super global and sets them into a new array which will be used to create
* new links to the referred posts to display adjacent posts
*
* @return (array)$queryVars
*/
public function referrerPermalinkVariables();
}
RequestReferrerHandler.php
<?php
namespace PG\Single\Post\Navigation;
/**
* Test set values against the super global given. Returns conditional properties
* which is boolean values. true is returned on success and false on failure
*
* @param $superGlobalVar Super global to test the values against
* @param (string) $authorReferrer
* @param (string) $dateReferrer
* @param (string) $searchReferrer
* @param (string) $taxReferrer
*/
class RequestReferrerHandler implements RequestReferrerHandlerInterface
{
/**
* @var (array) $superGlobalVar
*/
protected $superGlobalVar;
/**
* @var (string) $authorReferrer
*/
protected $authorReferrer;
/**
* @var (string) $dateReferrer
*/
protected $dateReferrer;
/**
* @var (string) $searchReferrer
*/
protected $searchReferrer;
/**
* @var (string) $taxReferrer
*/
protected $taxReferrer;
/**
* Public constructor method
*
* @param $superGlobalVar Super global to get data from
* @param $authorReferrer Query variable from author referrer to test
* @param $dateReferrer Query variable from date referrer to test
* @param $searchReferrer Query variable from search referrer to test
* @param $taxReferrer Query variable from taxonomy referrer to test
*/
public function __construct($superGlobalVar = null, $authorReferrer= null, $dateReferrer = null, $searchReferrer = null, $taxReferrer = null )
{
$this->superGlobalVar = $superGlobalVar;
$this->authorReferrer = $authorReferrer;
$this->dateReferrer = $dateReferrer;
$this->searchReferrer = $searchReferrer;
$this->taxReferrer = $taxReferrer;
}
/**
* Setter setSuperGlobalVar
* @param $superGlobalVar
* @return $this
*/
public function setSuperGlobalVar($superGlobalVar)
{
$this->superGlobalVar = $superGlobalVar;
return $this;
}
/**
* Returns an array of super global variables
* @return (array) $this->superGlobalVar
*/
public function getSuperGlobalVar()
{
return $this->superGlobalVar;
}
/**
* Setter setAuthorReferrer
* @param $authorReferrer
* @return $this
*/
public function setAuthorReferrer($authorReferrer)
{
$this->authorReferrer = $authorReferrer;
return $this;
}
/**
* Returns the value of the $authorReferrer property
* @return (array) $this->authorReferrer
*/
public function getAuthorReferrer()
{
return $this->authorReferrer;
}
/**
* Setter setDateReferrer
* @param $dateReferrer
* @return $this
*/
public function setDateReferrer($dateReferrer)
{
$this->dateReferrer = $dateReferrer;
return $this;
}
/**
* Returns the value of the $dateReferrer property
* @return (array) $this->dateReferrer
*/
public function getDateReferrer()
{
return $this->dateReferrer;
}
/**
* Setter setSearchReferrer
* @param $searchReferrer
* @return $this
*/
public function setSearchReferrer($searchReferrer)
{
$this->searchReferrer = $searchReferrer;
return $this;
}
/**
* Returns the value of the $searchReferrer property
* @return (array) $this->searchReferrer
*/
public function getSearchReferrer()
{
return $this->searchReferrer;
}
/**
* Setter setTaxReferrer
* @param $taxReferrer
* @return $this
*/
public function setTaxReferrer($taxReferrer)
{
$this->taxReferrer = $taxReferrer;
return $this;
}
/**
* Returns the value of the $taxReferrer property
* @return (array) $this->taxReferrer
*/
public function getTaxReferrer()
{
return $this->$taxReferrer;
}
/**
* Test $authorReferrer against $superGlobalVar
*
* @return (bool) true on success or false on failure
*/
public function isAuthorReferrer()
{
if ($this->authorReferrer && isset($this->superGlobalVar[$this->authorReferrer])) {
$isAuthorReferrer = true;
} else {
$isAuthorReferrer = false;
}
return $isAuthorReferrer;
}
/**
* Test $authorReferrer against $superGlobalVar
*
* @return (bool) true on success or false on failure
*/
public function isDateReferrer()
{
if ($this->dateReferrer && isset($this->superGlobalVar[$this->dateReferrer])) {
$isDateReferrer = true;
} else {
$isDateReferrer = false;
}
return $isDateReferrer;
}
/**
* Test $authorReferrer against $superGlobalVar
*
* @return (bool) true on success or false on failure
*/
public function isSearchReferrer()
{
if ($this->searchReferrer && isset($this->superGlobalVar[$this->searchReferrer])) {
$isSearchReferrer = true;
} else {
$isSearchReferrer = false;
}
return $isSearchReferrer;
}
/**
* Test $authorReferrer against $superGlobalVar
*
* @return (bool) true on success or false on failure
*/
public function isTaxReferrer()
{
if ($this->taxReferrer && isset($this->superGlobalVar[$this->taxReferrer])) {
$isTaxReferrer = true;
} else {
$isTaxReferrer = false;
}
return $isTaxReferrer;
}
/**
* Conditional which check if the current post is a referred post
*
* @return (bool) true on success or false on failure
*/
public function isReferredPost()
{
if ($this->isAuthorReferrer() || $this->isDateReferrer() || $this->isSearchReferrer() || $this->isTaxReferrer()) {
$isReferredPost = true;
} else {
$isReferredPost = false;
}
return $isReferredPost;
}
/**
* Sets the values retrieves from the super global to an array
* Will be passed as query arguments in a new instance of WP_Query
*
* @return (array) $queryArgs
*/
public function referrerQueryArguments()
{
if (!$this->isReferredPost())
return null;
if ($this->isAuthorReferrer()) {
$queryArgs = ['author' => (int) htmlspecialchars($this->superGlobalVar[$this->authorReferrer])];
} elseif ($this->isDateReferrer()) {
$queryArgs = ''; //@TODO Date archives
} elseif ($this->isSearchReferrer()) {
$queryArgs = ['s' => (string) htmlspecialchars($this->superGlobalVar[$this->searchReferrer])];
} elseif ($this->isTaxReferrer()) {
$terms = explode(' ', $this->superGlobalVar[$this->taxReferrer]) ;
$queryArgs = ['tax_query' => [
[
'taxonomy' => (string) htmlspecialchars($terms[0]),
'terms' => (int) htmlspecialchars($terms[1]),
'include_children' => false
]
]];
}
return (array) $queryArgs;
}
/**
* Gets the values from the super global and sets them into a new array which will be used to create
* new links to the referred posts to display adjacent posts
*
* @return (array)$queryVars
*/
public function referrerPermalinkVariables()
{
if (!$this->isReferredPost())
return null;
if ($this->isAuthorReferrer()) {
$queryVars = [$this->authorReferrer => htmlspecialchars($this->superGlobalVar[$this->authorReferrer])];
} elseif ($this->isDateReferrer()) {
$queryVars = ''; //@TODO Date archives
} elseif ($this->isSearchReferrer()) {
$queryVars = [$this->searchReferrer => str_replace( ' ', '+', htmlspecialchars($this->superGlobalVar[$this->searchReferrer]))];
} elseif ($this->isTaxReferrer()) {
$queryVars = [$this->taxReferrer => str_replace( ' ', '+', htmlspecialchars($this->superGlobalVar[$this->taxReferrer]))];
}
return (array)$queryVars;
}
}
The class can then be used on its own (I will use this in a system which will return posts from a new instance of WP_Query
in Wordpress according to the referrers set) and can easily be isolation tested. The following code
$a = new RequestReferrerHandler($_GET, 'aq', 'dq', 'sq', 'tq');
?><pre><?php var_dump($a->referrerQueryArguments()); ?></pre><?php
?><pre><?php var_dump($a->referrerPermalinkVariables()); ?></pre><?php
renders
array(1) {
["author"]=>
int(2)
}
and
array(1) {
["aq"]=>
string(1) "2"
}
when I'm on a single post page which was referred from an author page
Hope someone will find this useful :-)