My perfect shopping cart class Here is my shopping cart class! I want it to be perfect and never to have to implement it.

What improvements would you do?

New : IComparable

interface IComparable
{
    public function equals($object);
}


New : ICartItem

interface ICartItem extends IComparable
{
    public function setQuantity($quantity);
    public function getQuantity();
}


New : ICart

interface ICart
{
    public function add(ICartItem $item);
    public function remove(ICartItem $item);
    public function getQuantity(ICartItem $item);
    public function setQuantity(ICartItem $item, $quantity);
    public function isEmpty();
    public function getItems();
    public function clear();
}


New : Cart

class SessionCart implements ICart
{
    const IDENTIFIER = '_CART_';
    protected $items;

    public function __construct(&amp;$container = null)
    {
        if (is_null($container)) {
            if (session_id() == '') {
                session_start();
            }
            if (!isset($_SESSION[self::IDENTIFIER])) {
                $_SESSION[self::IDENTIFIER] = array();
            }
            $container = &amp; $_SESSION[self::IDENTIFIER];
        }
        $this-&gt;items = &amp; $container;
    }

    public function add(ICartItem $item)
    {
        $index = $this-&gt;getIndexOfItem($item);

        if ($index == -1) {
            $this-&gt;items[] = $item;

        } else {
            $item = $this-&gt;items[$index];
            $item-&gt;setQuantity($item-&gt;getQuantity() + 1);
        }

        return $item-&gt;getQuantity();
    }

    public function remove(ICartItem $item)
    {
        $index = $this-&gt;getIndexOfItem($item);

        if ($index == -1) {
            throw new Exception('The item isn\'t inside the cart.');
        }

        $item = $this-&gt;items[$index];

        $quantity = $item-&gt;getQuantity() - 1;

        if ($quantity &gt; 0) {
            $item-&gt;setQuantity($quantity);

        } else {
            unset($this-&gt;items[$index]);
        }
        return $quantity;
    }

    public function getQuantity(ICartItem $item)
    {
        $index = $this-&gt;getIndexOfItem($item);

        if ($index == -1) {
            return 0;

        } else {
            return $this-&gt;items[$index]-&gt;getQuantity();
        }
    }

    public function setQuantity(ICartItem $item, $quantity)
    {
        if (($quantity = (int)$quantity) &lt; 1) {
            throw new Exception('A positive quantity is required.');
        }

        $index = $this-&gt;getIndexOfItem($item);

        if ($index == -1) {
            $item-&gt;setQuantity($quantity);
            $this-&gt;items[] = $item;

        } else {
            $item = $this-&gt;items[$index];
            $item-&gt;setQuantity($quantity);
        }

        return $item-&gt;getQuantity();
    }

    public function isEmpty()
    {
        return empty($this-&gt;items);
    }

    public function getItems()
    {
        return $this-&gt;items;
    }

    public function clear()
    {
        $this-&gt;items = array();
    }

    private function getIndexOfItem(ICartItem $item)
    {
        foreach ($this-&gt;items as $key =&gt; $value) {
            if ($item-&gt;equals($value)) {
                return $key;
            }
        }
        return -1;
    }
}
General

You should reconsider the naming of some methods. For example,

interface IComparable
{
    public function equals($obj);
}


is more clear about what it really does (comments are omitted to save space; of course, proper DocBlock comments should always be included with your code). This way, you can easily add other methods like isLessThan, isGreaterOrEqual, and so on. A method named compareTo as in

$a-&gt;compareTo($b)


I'd expect to be a valid callback for usort, thus returning -1 on $a &lt; $b, 0 on $a == $b, or 1 on $a &gt; $b.

Cart

One day, you might want to store your cart items in a database, so using $_SESSION directly is not optimal. I'd define a cart interface:

interface ICart
{
    public function add(IComparable $obj);
    public function remove(IComparable $obj);
    public function getQuantity(IComparable $obj);
    public function setQuantity(IComparable $obj, $qty);
    public function isEmpty();
    public function getAll();
    public function clear();
}

class Cart implements ICart
{
    const IDENTIFIER = '_CART_';
    protected $container;

    public function __construct(&amp;$container = null)
    {
        if (is_null($container)) {
            if (session_id() == '') {
                session_start();
            }
            if (!isset($_SESSION[self::IDENTIFIER])) {
                $_SESSION[self::IDENTIFIER] = array();
            }
            $container = &amp;$_SESSION[self::IDENTIFIER];
        }
        $this-&gt;container = &amp;$container;
    }
}


With this approach, you can always build other cart implementations, like a database-aware one. The Cart class can be provided with an array, which will be used to store the data. If you want, you can pass in the $_SESSION superglobal directly, or use any other array. If omitted, a kind of namespace within the session variables (self::IDENTIFIER, '_CART_') is used to store the data.

Since the session - if needed - is started within the constructor, you can rely on its existence in the subsequent methods. BTW: the existence of the $_SESSION array does not guarantee that a session has been started! Use session_id() to check that instead.)

Internal Data Structure

Since your data array can be accessed from outside the cart, the current data structure is prone to get out of sync. It is more robust to swap the indices. Then you can replace getIndex() with getEntry(), which makes the handling much easier.

class Cart implements ICart
{
    ... // see above

    public function add(IComparable $obj)
    {
        $entry = &amp;$this-&gt;getEntry($obj);
        $entry['quantity']++;

        return $entry['quantity'];
    }

    public function remove(IComparable $obj)
    {
        $entry = &amp;$this-&gt;getEntry($obj);
        $entry['quantity'] = max(0, --$entry['quantity']);

        return $entry['quantity'];
    }

    public function getQuantity(IComparable $obj)
    {
        $entry = &amp;$this-&gt;getEntry($obj);
        return $entry['quantity'];
    }

    public function setQuantity(IComparable $obj, $qty)
    {
        $entry = &amp;$this-&gt;getEntry($obj);
        if ($entry['quantity'] &gt; 0) {
            $entry['quantity'] = (int) $qty;
        }
        return $entry['quantity'];
    }

    public function isEmpty()
    {
        $total = 0;
        foreach ($this-&gt;container as $entry) {
            $total += $entry['quantity'];
        }
        return $total &lt;= 0;
    }

    public function getAll()
    {
        $cart = array();
        foreach ($this-&gt;container as $entry) {
            if ($entry['quantity'] &gt; 0) {
                $cart[] = $entry;
            }
        }
        return $cart;
    }

    public function clear()
    {
        $this-&gt;container = array();
    }

    private function &amp;getEntry(IComparable $obj)
    {
        foreach ($this-&gt;container as &amp;$entry) {
            if ($obj-&gt;equals($entry['item'])) {
                return $entry;
            }
        }
        $entry = array(
            'item' =&gt; $obj,
            'quantity' =&gt; 0
        );
        $this-&gt;container[] = &amp;$entry;
        return $entry;
    }
}


As you can see, all methods (excl. isEmpty()) become much simpler.