0

I was asked to refactor some almost ureadable spaghetti code into object-oriented architecture.

I have some doubts regarding a class that I designed. Here is the class' skeleton:

require_once 'inc/FeaturedCarousel/Carousel.php';
class FeaturedCarousel
{
    private $currentCategory;
    private $allFeaturedAdvertisers = array();
    private $metaData = array();
    private $featuredAdvertisersIds = array();
    private $dataForDisplay = array();

    public function __construct($currentCategory)
    {
        $this->currentCategory = $currentCategory;
        $this->getAllFeaturedAdvertisers();
        $this->getAllAdvertisersMetaData();
        $this->getFeaturedAdvertisersInCurrentCategory();
        $this->getDataForDisplay();
    }    

    private function getAllFeaturedAdvertisers()
    {
    }

    private function getAllAdvertisersMetaData()
    {
    }

    private function getFeaturedAdvertisersInCurrentCategory()
    {             
    }

    private function getDataForDisplay()
    {     
    }

    public function displayCarousel()
    {
        new FeaturedCarousel\Carousel($this->dataForDisplay);
    }      
}

The class is initiated like this:

$carousel = new FeaturedCarousel($currentCategory);
$carousel->displayCarousel();

The class' role is to prepare data for display, and then display it using an external Carousel class that is instantiated in the displayCarousel() method.

There may be major aspects of it that should refactored. Please note that I have already implemented all the methods so there is no need to focus on their implementation.

Here are my questions -

  1. All methods required to generate data for display are trigerred one by one in the constructor. Each of these methods generates a private property required by the method called next. Therefore, if any of the methods fails, there is really no point in calling subsequent methods. How should I address those possible method failures?

  2. The displayCarousel() method instantiates another class that is responsible for the actual rendering of HTML content. At the moment the Carousel class is just required and instantiated in the method call. Should the Carousel class be 'injected' into my class before it is used?

  • please don't cross-post: stackoverflow.com/questions/29239416/… – gnat Mar 24 '15 at 20:51
  • 1
    @gnat The other post has now been deleted. I kept it live for a while longer due to very interesting discussion that the the topic started. But as I said - no cross-posting anymore – luqo33 Mar 24 '15 at 21:10
1

if any of the methods fails, there is really no point in calling subsequent methods. How should I address those possible method failures?

Throw an exception.

Should the Carousel class be 'injected' into my class before it is used?

Yes, if there is more than one implementation of Carousel (including the possibility of a mock object for unit testing purposes). Then you will inject an ICarousel.

  • you're truly the persion I've been waiting for this whole day across several Stack sites. Much appreciated! :) – luqo33 Mar 24 '15 at 21:00

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.