1
\$\begingroup\$

I'm trying to get my head around the OOP principles and coding my own classes. As a means to learn, I have decided to convert a couple of functions I have written in Wordpress to OOP classes. These functions work together in order to output the correct post links on single pages according to referrers (4 of them) set in the URL.

This is the setup with a basic workflow (The workflow can change as I go along):

enter image description here

4 query variables are set to the URL according to archive page, ie, one query variable for taxonomy pages, one query variable set for the author pages and so one. No page can ever have more than one custom query variable. This 4 variables is retrieved by my first class and checked against a given global variable, in this case $_GET. I have not hardcoded the 4 variables in my class, and this goes for $_GET as well to keep the class testable. If the value exists in the URL, the key/value pair is returned through the has* methods. These methods return null if no match is found. (this is raw data which will be sanitized/escaped by the classes that will use this data)

Here is the full class:

<?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
{
    /**
     * @since 1.0.0
     * @access protected
     * @var (array) $superGlobalVar
    */
    protected $superGlobalVar;

    /**
     * @since 1.0.0
     * @access protected
     * @var (string) $authorReferrer
    */
    protected $authorReferrer;

    /**
     * @since 1.0.0
     * @access protected
     * @var (string) $dateReferrer
    */
    protected $dateReferrer;

    /**
     * @since 1.0.0
     * @access protected
     * @var (string) $searchReferrer
    */
    protected $searchReferrer;

    /**
     * @since 1.0.0
     * @access protected
     * @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.
     *
     * @since 1.0.0
     * @param $superGlobalVar
     * @return $this
     */
    public function setSuperGlobalVar($superGlobalVar)
    {
        $this->superGlobalVar = $superGlobalVar;
        return $this;
    }   

    /**
     * Returns an array of super global variables.
     *
     * @since 1.0.0
     * @return (array) $this->superGlobalVar
    */ 
    public function getSuperGlobalVar()
    {
        return $this->superGlobalVar;
    }

    /**
     * Setter setAuthorReferrer
     *
     * @since 1.0.0
     * @param $authorReferrer
     * @return $this
     */
    public function setAuthorReferrer($authorReferrer)
    {
        $this->authorReferrer = $authorReferrer;
        return $this;
    }   

    /**
     * Returns the value of the $authorReferrer property.
     *
     * @since 1.0.0
     * @return (array) $this->authorReferrer
    */ 
    public function getAuthorReferrer()
    {
        return $this->authorReferrer;
    }

    /**
     * Setter setDateReferrer.
     *
     * @since 1.0.0
     * @param $dateReferrer
     * @return $this
     */
    public function setDateReferrer($dateReferrer)
    {
        $this->dateReferrer = $dateReferrer;
        return $this;
    }   

    /**
     * Returns the value of the $dateReferrer property.
     *
     * @since 1.0.0
     * @return (array) $this->dateReferrer
    */ 
    public function getDateReferrer()
    {
        return $this->dateReferrer;
    }

    /**
     * Setter setSearchReferrer.
     *
     * @since 1.0.0
     * @param $searchReferrer
     * @return $this
     */
    public function setSearchReferrer($searchReferrer)
    {
        $this->searchReferrer = $searchReferrer;
        return $this;
    }   

    /**
     * Returns the value of the $searchReferrer property.
     *
     * @since 1.0.0
     * @return (array) $this->searchReferrer
    */ 
    public function getSearchReferrer()
    {
        return $this->searchReferrer;
    }

    /**
     * Setter setTaxReferrer.
     *
     * @since 1.0.0
     * @param $taxReferrer
     * @return $this
     */
    public function setTaxReferrer($taxReferrer)
    {
        $this->taxReferrer = $taxReferrer;
        return $this;
    }   

    /**
     * Returns the value of the $taxReferrer property.
     *
     * @since 1.0.0
     * @return (array) $this->taxReferrer
    */ 
    public function getTaxReferrer()
    {
        return $this->$taxReferrer;
    }

    /**
     * Test $authorReferrer against $superGlobalVar.
     *
     * @since 1.0.0
     * @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
     *
     * @since 1.0.0
     * @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.
     *
     * @since 1.0.0
     * @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.
     *
     * @since 1.0.0
     * @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.
     *
     * @since 1.0.0
     * @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;
    }

    /**
     * Return the value from the super global when the current post is a post referred from
     * an author archive page.
     *
     * @since 1.0.0
     * @return (array) $authorReferrerValue
     */ 
    public function hasAuthorReferrerValue()
    {
        if ($this->isAuthorReferrer()) {
            $authorReferrerValue = [$this->authorReferrer => $this->superGlobalVar[$this->authorReferrer]];
        } else {
            $authorReferrerValue = null;
        }
        return $authorReferrerValue;
    }

    /**
     * Return the value from the super global when the current post is a post referred from
     * a date archive page.
     *
     * @since 1.0.0
     * @return (array) $dateReferrerValue
     */ 
    public function hasDateReferrerValue()
    {
        if ($this->isDateReferrer()) {
            $dateReferrerValue = [$this->dateReferrer => $this->superGlobalVar[$this->dateReferrer]];
        } else {
            $dateReferrerValue = null;
        }
        return $dateReferrerValue;
    }

    /**
     * Return the value from the super global when the current post is a post referred from
     * a search page.
     *
     * @since 1.0.0
     * @return (array) $searchReferrerValue
     */ 
    public function hasSearchReferrerValue()
    {
        if ($this->isSearchReferrer()) {
            $searchReferrerValue = [$this->searchReferrer => $this->superGlobalVar[$this->searchReferrer]];
        } else {
            $searchReferrerValue = null;
        }
        return $searchReferrerValue;
    }

    /**
     * Return the value from the super global when the current post is a post referred from
     * a taxonomy archive page.
     *
     * @since 1.0.0
     * @return (array) $taxReferrerValue
     */ 
    public function hasTaxReferrerValue()
    {
        if ($this->isTaxReferrer()) {
            $taxReferrerValue = [$this->taxReferrer => $this->superGlobalVar[$this->taxReferrer]];
        } else {
            $taxReferrerValue = null;
        }
        return $taxReferrerValue;
    }

}

This is how I use this class:

$b = new RequestReferrerHandler($_GET, 'aq', 'dq', 'sq', 'tq');
?><pre><?php var_dump($b->hasAuthorReferrerValue()); ?></pre><?php
?><pre><?php var_dump($b->hasDateReferrerValue()); ?></pre><?php
?><pre><?php var_dump($b->hasSearchReferrerValue()); ?></pre><?php
?><pre><?php var_dump($b->hasTaxReferrerValue()); ?></pre><?php

For testing purposes you can inject something like ['aq' => '1'] into the class instead of $_GET.

This is where I'm stuck now and have no idea how to move on. I need to construct two classes which will both use the same methods from the class above, one class that will construct query arguments from the has* methods from the above class, and one class will create query_vars also from the has* methods from the above class that will be used to construct new post links

So, in short, both classes will make use of the exact same for methods from the above class:

hasAuthorReferrerValue();
hasDateReferrerValue();
hasSearchReferrerValue();
hasTaxReferrerValue();

Just as an example, here is an example of how the two classes should look like. (I have omitted some of the methods here to make the code more manageable)

ClassA

<?php
namespace PG\Single\Post\Navigation;

class ClassA //Just a generic name for testing purposes. Will also implement ClassAInterface
{
    protected $handler;

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

    public function santizeAuthor() 
    {
        $author = $this->handler->hasAuthorReferrerValue(); // Value will be either null or single key/value pair array. Example ['aq' => '1']

        if ($author) {
            $author = array_values($author);
            $author = ['author' => (int)htmlspecialchars($author[0])]; //Will output ['author' => 1]
        }

        return $author; //Returns null or the array ['author' => 1]
    }

    public function santizeDate() 
    {
        $date = $this->handler->hasDateReferrerValue();

        if ($date) {
            // @TODO Still to work out
        }

        return $date;
    }

    //etc

    public function queryArguments() // Will be used in the controller class ClassC
    {
        $queryArgs = null;

        if ($this->santizeAuthor()) {

            $queryArgs = $this->santizeAuthor();

        } elseif ($this->santizeDate) {

            $queryArgs = $this->santizeDate();

        } // etc
        return $queryArgs; //Will return null if all 4 conditions fail or return the value from the one that returns true
    }

}

ClassB

<?php
namespace PG\Single\Post\Navigation;

class ClassB //Just a generic name for testing purposes. Will also implement ClassBInterface
{
    protected $handler;

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

    public function santizeAuthor() 
    {
        $author = $this->handler->hasAuthorReferrerValue(); // Value will be either null or single key/value pair array. Example ['aq' => '1']

        if ($author) {
            foreach ($author as $k=>$v)
                $author[htmlspecialchars($k)] = (int)htmlspecialchars($v);
        }

        return $author; //Returns null or the array ['aq' => 1]
    }

    public function santizeDate() 
    {
        $date = $this->handler->hasDateReferrerValue();

        if ($date) {
            // @TODO Still to work out
        }

        return $date;
    }

    //etc

    public function queryVars() // Will be used in the controller class ClassC
    {
        $queryVars = null;

        if ($this->santizeAuthor()) {

            $queryVars = $this->santizeAuthor();

        } elseif ($this->santizeDate) {

            $queryVars = $this->santizeDate();

        } // etc
        return $queryVars; //Will return null if all 4 conditions fail or return the value from the one that returns true
    }

}

The queryArguments() method from ClassA and the queryVars() method from ClassB will be used in other classes (or one controller class).

My total lack of proper knowledge coming into OOP, confusion with separation of concerns, encapsulation, SOLID principles and keeping class testable have me second guessing my code, and I do feel I am missing something.

Is there anyway I can optimize the above? I am not asking for any type of code rewriting, all I need is proper pointers and ideas on optimizing this to bring it up to standard if it is not. It would be a real plus if anyone can give code samples, something like an outline skeleton

\$\endgroup\$
1
\$\begingroup\$

Naming

  • hasX (eg hasAuthorReferrerValue) usually returns a boolean (as in, does this instance have an author?). getX would be more appropriate.
  • queryArguments and queryVars do exactly the same thing. Why do they have different names? Equal things should have equal names.
  • ClassA and ClassB are just not acceptable names.

OOP

  • I'm still unsure why you need RequestReferrerHandlerInterface. Yes, it's a part of SOLID, but you don't have to abide by that 100% all the time. If there is no other possible implementation, you can remove it.
  • I'm not sure why you have ClassA and ClassB, but they are obviously quite similar. Either use inheritance (if the implementation of enough methods is exactly the same), and/or an interface.

Misc

  • for a generic input cleaning function you definitely also want to encode quotes: htmlspecialchars($string, ENT_QUOTES, 'UTF-8'). Imagine eg <a href='[user input]'>test</a> and a user inputting ' onload=evil(); foo='.
\$\endgroup\$
  • \$\begingroup\$ Yes, I know my class names are unexceptable, also commented that it is only for testing, so it can be ignored. queryArguments and queryVars are different as the returned arrays will differ. How possible is it for me to move ClassA and ClassB into my referrer class, will that still be acceptable \$\endgroup\$ – Pieter Goosen Feb 27 '15 at 9:37
  • \$\begingroup\$ Will my class then not do to much in terms of that a class should only perform one task \$\endgroup\$ – Pieter Goosen Feb 27 '15 at 9:39

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.