HiveBrain v1.2.0
Get Started
← Back to all entries
patternphpMinor

Cart Class feedback

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
feedbackcartclass

Problem

I have written a cart Class, it use session to store data. Is the code well implemented or could been better? You can rewrite my code if you can.

Each item have a number of options with price

Each Option can have many extras or without extras

For example:

Item 1 -> (Option ID 1)

Item 2 -> (Option ID 3, Option ID 4)

Option ID 4 => (Extras ID: 1,5,7,8)

cartName = $cartName;
        if (!isset($_SESSION[$this->cartName]))
        {
            $_SESSION[$this->cartName] = array();
        }
 }

   public  function addItem($itemid, $optionid)
     {
        if (array_key_exists($optionid, $_SESSION[$this->cartName]))
         {
            $_SESSION[$this->cartName][$optionid]['quantity']++;
            $_SESSION[$this->cartName]['LastUpdated']  = $optionid;
         }
            else
                {
                    $_SESSION[$this->cartName]['LastUpdated']  = $optionid;
                    $_SESSION[$this->cartName][$optionid] = array();
                    $_SESSION[$this->cartName][$optionid]['quantity'] = 1;
                    $_SESSION[$this->cartName][$optionid]['item_id'] = $itemid;
                    $_SESSION[$this->cartName][$optionid]['extras_id'] = array();
                }
     }

    public function addExtra($optionid,$extraid)
    {
        if (array_key_exists($optionid, $_SESSION[$this->cartName]))
            {
               if (!in_array($extraid, $_SESSION[$this->cartName][$optionid]['extras_id']))
               {
                   $_SESSION[$this->cartName][$optionid]['extras_id'][] = $extraid;
                }
            }
    }

}

$test =  new Cart("shopCart");

$test->addItem(5,2);
$test->addExtra(2,10);
$test->addExtra(2,20);
$test->addExtra(2,30);

$test->addItem(5,4);

echo "";
print_r($_SESSION);
echo "";

Solution

OK, bear with me here, but the formatting stinks.

I won't rewrite the code for you, but I will continue to provide the information that I can to enable you to do so, should you (based on existing knowledge, or in any of my opinions being finely refuted by others) choose to disagree, then don't implement any of it.

I was about to refuse even to read the code until the formatting (proper indentation) was applied. Formatting is one of the fundamental aspects of our job, yet (seeing as it isn't mentioned as a target of the desired review) is something you are oblivious to, can't be bothered to correct the copy+paste anomalies of, or otherwise have reasoning betwixt these matters that inhibits any action on your part to correct it.

I won't judge harshly, however, so let us imagine you do need advice on this before even thinking about moving on. It is not only necessary to format code in a easily readable fashion, consistency of formatting is expected too; in fact, in this role, consistency to the highest degree possible is of the utmost importance in both what is to be done and how it is done. What indentation gives us is a clear view of the scope of the code we're currently looking at - without it, or when it is malformed, such can be either laborious to determine, misleading, or irritating (or any combination thereof!)

This is why (where curly braces are available for this purpose) I think it is a friendly approach to start and end execution scopes on dedicated lines; meaning, nothing before (other than the proper number of indentation spaces) or after the brace. And it's not just indentation, either: spacing, naming conventions, structure et cetera, all need attention.

On the note of naming convention: Is it a cart? Is it a CART? Oh, it's a cart...? Reductio ad absurdum aside, I would suggest you name this type (and its corresponding file) explicitly as ShoppingCart in order to make clear as to what it actually is. File size is of smaller concern these days meaning even web-code can speak for itself most times - if size is a concern, then I'm sure you could find a tool for PHP that 'minifies' your release source, still enabling you (and others) to work on self-explanatory code.

PHP supports composite types and, by its nature and the nature of the task at hand, you start off using it but fail to extend it to be of any further use (even, if for nothing else, code-readability as mention above.) For instance, a ShoppingCartItem could expose the fields that are currently stored in a standard array (which is an error-prone method of doing so too, where maintenance and extensibility are concerned) relating the to actual item. A ShoppingCartItemExtra class could do the same for its properties. I'll let you chew further on that.

I'll just assume you were in too much of a hurry to add the ending php tag (?>).

I'll leave it there, with scope-formatted version of your code. :)

class Cart
{
    public function __construct($cartName)
    {
        if (!isset($_SESSION[$this->cartName]))
        {
        }
    }

    public function addItem($itemid, $optionid)
    {
        if (array_key_exists($optionid, $_SESSION[$this->cartName]))
        {
        }
        else
        {
        }
    }

    public function addExtra($optionid, $extraid)
    {
        if (array_key_exists($optionid, $_SESSION[$this->cartName]))
        {
            if (!in_array($extraid, $_SESSION[$this->cartName][$optionid]['extras_id']))
            {
            }
        }
    }
}

Code Snippets

class Cart
{
    public function __construct($cartName)
    {
        if (!isset($_SESSION[$this->cartName]))
        {
        }
    }

    public function addItem($itemid, $optionid)
    {
        if (array_key_exists($optionid, $_SESSION[$this->cartName]))
        {
        }
        else
        {
        }
    }

    public function addExtra($optionid, $extraid)
    {
        if (array_key_exists($optionid, $_SESSION[$this->cartName]))
        {
            if (!in_array($extraid, $_SESSION[$this->cartName][$optionid]['extras_id']))
            {
            }
        }
    }
}

Context

StackExchange Code Review Q#1784, answer score: 4

Revisions (0)

No revisions yet.