patternphpMinor
My perfect shopping cart class
Viewed 0 times
perfectcartshoppingclass
Problem
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
New : ICartItem
New : ICart
New : Cart
```
class SessionCart implements ICart
{
const IDENTIFIER = '_CART_';
protected $items;
public function __construct(&$container = null)
{
if (is_null($container)) {
if (session_id() == '') {
session_start();
}
if (!isset($_SESSION[self::IDENTIFIER])) {
$_SESSION[self::IDENTIFIER] = array();
}
$container = & $_SESSION[self::IDENTIFIER];
}
$this->items = & $container;
}
public function add(ICartItem $item)
{
$index = $this->getIndexOfItem($item);
if ($index == -1) {
$this->items[] = $item;
} else {
$item = $this->items[$index];
$item->setQuantity($item->getQuantity() + 1);
}
return $item->getQuantity();
}
public function remove(ICartItem $item)
{
$index = $this->getIndexOfItem($item);
if ($index == -1) {
throw new Exception('The item isn\'t inside the cart.');
}
$item = $this->items[$index];
$quantity = $item->getQuantity() - 1;
if ($quantity > 0) {
$item->setQuantity($quantity);
} else {
unset($this->items[$index]);
}
return $q
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(&$container = null)
{
if (is_null($container)) {
if (session_id() == '') {
session_start();
}
if (!isset($_SESSION[self::IDENTIFIER])) {
$_SESSION[self::IDENTIFIER] = array();
}
$container = & $_SESSION[self::IDENTIFIER];
}
$this->items = & $container;
}
public function add(ICartItem $item)
{
$index = $this->getIndexOfItem($item);
if ($index == -1) {
$this->items[] = $item;
} else {
$item = $this->items[$index];
$item->setQuantity($item->getQuantity() + 1);
}
return $item->getQuantity();
}
public function remove(ICartItem $item)
{
$index = $this->getIndexOfItem($item);
if ($index == -1) {
throw new Exception('The item isn\'t inside the cart.');
}
$item = $this->items[$index];
$quantity = $item->getQuantity() - 1;
if ($quantity > 0) {
$item->setQuantity($quantity);
} else {
unset($this->items[$index]);
}
return $q
Solution
General
You should reconsider the naming of some methods. For example,
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
I'd expect to be a valid callback for
Cart
One day, you might want to store your cart items in a database, so using
With this approach, you can always build other cart implementations, like a database-aware one. The
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
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
As you can see, all methods (excl.
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->compareTo($b)I'd expect to be a valid callback for
usort, thus returning -1 on $a $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(&$container = null)
{
if (is_null($container)) {
if (session_id() == '') {
session_start();
}
if (!isset($_SESSION[self::IDENTIFIER])) {
$_SESSION[self::IDENTIFIER] = array();
}
$container = &$_SESSION[self::IDENTIFIER];
}
$this->container = &$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 = &$this->getEntry($obj);
$entry['quantity']++;
return $entry['quantity'];
}
public function remove(IComparable $obj)
{
$entry = &$this->getEntry($obj);
$entry['quantity'] = max(0, --$entry['quantity']);
return $entry['quantity'];
}
public function getQuantity(IComparable $obj)
{
$entry = &$this->getEntry($obj);
return $entry['quantity'];
}
public function setQuantity(IComparable $obj, $qty)
{
$entry = &$this->getEntry($obj);
if ($entry['quantity'] > 0) {
$entry['quantity'] = (int) $qty;
}
return $entry['quantity'];
}
public function isEmpty()
{
$total = 0;
foreach ($this->container as $entry) {
$total += $entry['quantity'];
}
return $total container as $entry) {
if ($entry['quantity'] > 0) {
$cart[] = $entry;
}
}
return $cart;
}
public function clear()
{
$this->container = array();
}
private function &getEntry(IComparable $obj)
{
foreach ($this->container as &$entry) {
if ($obj->equals($entry['item'])) {
return $entry;
}
}
$entry = array(
'item' => $obj,
'quantity' => 0
);
$this->container[] = &$entry;
return $entry;
}
}As you can see, all methods (excl.
isEmpty()) become much simpler.Code Snippets
interface IComparable
{
public function equals($obj);
}$a->compareTo($b)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(&$container = null)
{
if (is_null($container)) {
if (session_id() == '') {
session_start();
}
if (!isset($_SESSION[self::IDENTIFIER])) {
$_SESSION[self::IDENTIFIER] = array();
}
$container = &$_SESSION[self::IDENTIFIER];
}
$this->container = &$container;
}
}class Cart implements ICart
{
... // see above
public function add(IComparable $obj)
{
$entry = &$this->getEntry($obj);
$entry['quantity']++;
return $entry['quantity'];
}
public function remove(IComparable $obj)
{
$entry = &$this->getEntry($obj);
$entry['quantity'] = max(0, --$entry['quantity']);
return $entry['quantity'];
}
public function getQuantity(IComparable $obj)
{
$entry = &$this->getEntry($obj);
return $entry['quantity'];
}
public function setQuantity(IComparable $obj, $qty)
{
$entry = &$this->getEntry($obj);
if ($entry['quantity'] > 0) {
$entry['quantity'] = (int) $qty;
}
return $entry['quantity'];
}
public function isEmpty()
{
$total = 0;
foreach ($this->container as $entry) {
$total += $entry['quantity'];
}
return $total <= 0;
}
public function getAll()
{
$cart = array();
foreach ($this->container as $entry) {
if ($entry['quantity'] > 0) {
$cart[] = $entry;
}
}
return $cart;
}
public function clear()
{
$this->container = array();
}
private function &getEntry(IComparable $obj)
{
foreach ($this->container as &$entry) {
if ($obj->equals($entry['item'])) {
return $entry;
}
}
$entry = array(
'item' => $obj,
'quantity' => 0
);
$this->container[] = &$entry;
return $entry;
}
}Context
StackExchange Code Review Q#25671, answer score: 6
Revisions (0)
No revisions yet.