tags:

views:

111

answers:

3

How would you handle grouping different objects?

For example, lets say you have a book of sport cards and you want to organize and spit out data about that book of sport cards. Obviously each sport card should have its own class and be its own object, but how would you group different sets of sprots cards? What if you wanted to see how many cards (and even the cards themselves) that were made by a given manufacturer in a given year?

Should There be a class card_book that organizes each card with functions like

getTotalBySport($sport) // Gets total number of cards within a given sport

getTotalByPrice($min, $max) // Gets total number of cards within a given price range

getCardsBySport($sport) // Get an array (or whatever other container) of cards within a given sport

or should those functions be implemented directly into the cards class?

+3  A: 

You would have one class that describes the sports cards. From that class, you'd create objects representing each card, with all of its sundry details.

You may also want a class that represents a collection of sports cards to, well, manage a collection of sports cards.

Functions with in the sports card class (called methods in OOP speak) deal with managing the card's details. Functions within the collection class would deal with the collection.

Edit:

To expand on this:

The constructor for the card class would accept parameters to initialize the object when it's created. How you go about this depends on various issues. Since there's likely a lot of data, I would favor an associative array. This would make creating a bunch of objects easier.

You'd also likely want setter methods for all the info that a card represents, and of course getter methods in order to retrieve that info. Depending on your needs, you may do this for each piece of information, or chunk things up as reasonable. So, for example, maybe one method to add a year's worth of statistics, instead of one method for each particular type of stat. Meanwhile, something like the player's name would have it's own set of methods.

George Marian
+2  A: 

Will a card for each different sport have different properties that are unique to that sport or will all cards share the same properties? If different sports introduce different properties or behaviours then extend a SportCard base class for each. If not then simply make the sport's name a property of the SportCard.

You are on the right track with your separate management class to do stuff like getTotalBySport() - that's definitely the right place to do that.

Whether you extend SportCard or not will change the behaviour of your management class a bit.

Steve Claridge
+1 for bringing up the likely variation in data between sports.
George Marian
+3  A: 

I'd say follow the Single Responsibility Principle.

What you have is a SportsCard that does nothing on itself. It's just a data container.

class SportsCard
{
    protected $_price;
    protected $_sport;

    public function __construct($price, $sport)
    {
        $this->_price = $price;
        $this->_sport = $sport;
    }

    public function getPrice() { return $this->_price; }
    public function getSport() { return $this->_sport; }
}

The SportsCards have to be stored in a CardBook. If you think about a CardBook, then it doesn't do much, except for storing SportsCards (not Pokemon or Magic The Gathering cards), so let's just make it do that:

class CardBook extends SplObjectStorage
{
    public function attach($card)
    {
        if ($card instanceof SportsCard) {
            parent::attach($card);
        }
        return $this;
    }

    public function detach($card)
    {
        parent::detach($card);
    }
}

While there is nothing wrong with adding Finder Methods to the SportsCard set, you do not want the finder method's logic inside it as well. It is fine as long as you only have

public function findByPrice($min, $max);
public function findBySport($sport);

But the second you also add

public function findByPriceAndSport($sport, $min, $max);

you will either duplicate code or iterate over the collection twice. Fortunately, PHP offers an elegant solution for filtering collections with Iterators, so let's put the responsibility for filtering into separate classes as well:

abstract class CardBookFilterIterator extends FilterIterator
{
    public function __construct($cardBook)
    {
        if ($cardBook instanceof CardBook || $cardBook instanceof self) {
            parent::__construct($cardBook);
        } else {
            throw new InvalidArgumentException(
                'Expected CardBook or CardBookFilterIterator');
        }
    }
}

This class merely makes sure the child classes accept only children of itself or CardBooks. This is important because we are accessing methods from SportsCard in actual filters later and because CardBooks can contain only SportsCards, we make sure the iterated elements provide these methods. But let's look at a concrete Filter:

class SportCardsBySportFilter extends CardBookFilterIterator
{
    protected $sport = NULL;

    public function filterBySport($sport)
    {
        $this->_sport = $sport;
        return $this;
    }

    public function accept()
    {
        return $this->current()->getSport() === $this->_sport;
    }
}

FilterIterators work by extending the class and modifying it's accept() method with custom logic. If the method does not return FALSE, the currently iterated item is allowed to pass the filter. Iterators can be stacked, so you add Filter on Filter on top, each caring only for a single piece of filtering, which makes this very maintainable and extensible. Here is the second filter:

class SportCardsByPriceFilter extends CardBookFilterIterator
{
    protected $min;
    protected $max;

    public function filterByPrice($min = 0, $max = PHP_INT_MAX)
    {
        $this->_min = $min;
        $this->_max = $max;
        return $this;
    }

    public function accept()
    {
        return $this->current()->getPrice() > $this->_min &&
               $this->current()->getPrice() < $this->_max;
    }
}

No magic here. Just the accept method and the setter for the criteria. Now, to assemble it:

$cardBook = new CardBook;
$cardBook->attach(new SportsCard('10', 'Baseball'))
         ->attach(new SportsCard('40', 'Basketball'))
         ->attach(new SportsCard('50', 'Soccer'))
         ->attach(new SportsCard('20', 'Rugby'))
         ->attach(new SportsCard('30', 'Baseball'));

$filteredCardBook = new SportCardsBySportFilter($cardBook);
$filteredCardBook->setFilterBySport('Baseball');
$filteredCardBook = new SportCardsByPriceFilter($filteredCardBook);
$filteredCardBook->filterByPrice(20);

print_r( iterator_to_array($filteredCardBook) );

And this will give:

Array (
    [4] => SportsCard Object (
            [_price:protected] => 30
            [_sport:protected] => Baseball
        )
)

Combining filters from inside the CardBook is a breeze now. No Code duplication or double iteration anymore. Whether you keep the Finder Methods inside the CardBook or in a CardBookFinder that you can pass CardBooks into is up to you.

class CardBookFinder
{
    protected $_filterSet;
    protected $_cardBook;

    public function __construct(CardBook $cardBook)
    {
        $this->_cardBook = $this->_filterSet = $cardBook;
    }

    public function getFilterSet()
    {
        return $this->_filterSet;
    }

    public function resetFilterSet()
    {
        return $this->_filterSet = $this->_cardBook;
    }

These methods just make sure you can start new FilterSets and that you have a CardBook inside the Finder before using one of the finder methods:

    public function findBySport($sport)
    {
        $this->_filterSet = new SportCardsBySportFilter($this->getFilterSet());
        $this->_filterSet->filterBySport($sport);
        return $this->_filterSet;
    }

    public function findByPrice($min, $max)
    {
        $this->_filterSet = new SportCardsByPriceFilter($this->getFilterSet());
        $this->_filterSet->filterByPrice(20);
        return $this->_filterSet;
    }

    public function findBySportAndPrice($sport, $min, $max)
    {
        $this->findBySport($sport);
        $this->_filterSet = $this->findByPrice($min, $max);
        return $this->_filterSet;
    }

    public function countBySportAndPrice($sport, $min, $max)
    {
        return iterator_count(
            $this->findBySportAndPrice($sport, $min, $max));
    }
}

And there you go

$cardBookFinder = new CardBookFinder($cardBook);
echo $cardBookFinder->countBySportAndPrice('Baseball', 20, 100);
$cardBookFinder->resetFilterSet();
foreach($cardBookFinder->findBySport('Baseball') as $card) {
    echo "{$card->getSport()} - {$card->getPrice()}";
}

Feel free to adapt and improve for your own CardBook.

Gordon
+1 nice approach, though you pretty much spoon fed him. That said, why no setters in the SportsCard class? Is that intentional, or omitted for brevity?
George Marian
very good answer and very accurate. Anyway, isn't there an "error" in your "public function findBySport($sport)" : $this->_filterSet->filterBySport('Baseball'); shouln't be $this->_filterSet->filterBySport($sport); ?
Aif
@George Thanks. Yeah, I was in a spoonfeeding mood. Actually, it was a nice exercise for me too. Setters are omitted for brevity, though the OP can keep it this way if he wants SportsCard to be immutable.
Gordon
@Aif Thanks. Of course that is wrong. Fixed now
Gordon