2
\$\begingroup\$

I wanted to create a cart that I can easily add some item or simply help someone at the other end over the phone. I decided to create a cart that would store everything on MySQL instead of using $_SESSION.

I did not code the whole cart just in case that this idea is very very bad. But I wanted to show what I have done and know your feedback.

The MySQL table look like the following:

CREATE TABLE IF NOT EXISTS `checkouts` (
  `Id` int(11) NOT NULL AUTO_INCREMENT,
  `SessionId` varchar(30) NOT NULL,
  `LastTouchTime` int(11) NOT NULL,
  `ObjectSerialized` text NOT NULL,
  PRIMARY KEY (`Id`)
) ENGINE=InnoDB  DEFAULT CHARSET=utf8 AUTO_INCREMENT=0 ;

And this is the PHP class:

class Checkout
{
    static $KeepCartFor = 86400;
    static $dbCon = null;
    private $CheckoutId = null;
    private $Cart = array();
    private $PromoCode = null;
    private $SubTotal = 0.00;
            // Please Note that im using GST as the rst ( the cart is kinda made for   easy swap between canada store and usa
    private $Gst = 0.00; //Good And Services Taxes
    private $Pst = 0.00; //Provincial Tax
    private $Shipping = array("Method"=>null,"Cost"=>0.00);
    private $Total = 0.00;

    private $Customer = array("FirstName"=>null,"LastName"=>null,"Email"=>null,"Home"=>null,"Work"=>null,"Cell"=>null,"Fax"=>null,"Company"=>null,"Address1"=>null,"Address2"=>null,"Address3"=>null,"Country"=>null,"State"=>null,"City"=>null,"Zip"=>null,"isShippingSameAsBilling"=>true,"ShipFirstName"=>null,"ShipLastName"=>null,"ShipCompany"=>null,"ShipAddress1"=>null,"ShipAddress2"=>null,"ShipAddress3"=>null,"ShipCountry"=>null,"ShipState"=>null,"ShipCity"=>null,"ShipZip"=>null);

    public function Cart_AddItem($WebsiteId,$Qty = 1)
    {
        if(!isset($this->Cart[$WebsiteId]))
        {
            $this->Cart[$WebsiteId] = $this->_GetProductDetail($WebsiteId);
            $this->Cart[$WebsiteId]['Qty'] = $Qty;
        }
        else
            $this->Cart[$WebsiteId]['Qty'] += $Qty;
        $this->Shipping = array("Method"=>null,"Cost"=>0.00);
    }
    public function Cart_RemoveItem($WebsiteId,$Qty = null)
    {
        if(isset($this->Cart[$WebsiteId]))
            if(is_null($Qty))
                unset($this->Cart[$WebsiteId]);
            else
            {
                if($this->Cart[$WebsiteId]['Qty'] - $Qty <= 0)
                    unset($this->Cart[$WebsiteId]);
                else
                    $this -> Cart[$WebsiteId]['Qty'] -= $Qty;
            }
        $this->Shipping = array("Method"=>null,"Cost"=>0.00);
    }
    public function Cart_Emtpy()
    {
        $this -> Cart = array();
        $this -> SubTotal = 0.0;
        $this -> Gst = 0.00;
        $this -> Pst = 0.00;
        $this -> Total = 0.00;
    }
    public function Cart_GetItems($WithDetail = false)
    {
        if(!$WithDetail)
        {
            foreach($this->Cart as $WebsiteId => $Vars)
            {
                $Return[$WebsiteId] = $Vars['Qty'];
            }
            return $Return;
        }
        else
            return $this->Cart;
    }
    private function _RefrechCartVars()
    {
        $this->SubTotal = 0.00;
        $this->Gst = 0.00;
        $this->Pst = 0.00;
        $this->Total = 0.00;
        foreach($this->Cart as $WebsiteId => $Vars)
        {
            $this->SubTotal += $Vars['ActualPrice']*$Vars['Qty'];
        }
        if(($this->Customer['isShippingSameAsBilling']?$this->Customer['Country']:$this->Customer['ShipCountry']) == "United States")
            if(($this->Customer['isShippingSameAsBilling']?$this->Customer['State']:$this->Customer['ShipState']) == "New York")
                $this->Gst = ($this->SubTotal * 0.07) + (is_null($this->Shipping['Method'])?0.00:$this->Shipping['Cost']);
        $this->Total = $this->SubTotal + $this->Gst + (is_null($this->Shipping['Method'])?0.00:$this->Shipping['Cost']);
    }
    private function _GetProductDetail($WebsiteId)
    {
        $GetProductDetail = Checkout::$dbCon -> prepare("SELECT * FROM product WHERE id = :Id");
        $GetProductDetail -> bindValue(':Id',$WebsiteId);
        try{
            $GetProductDetail->execute();
        }catch(PDOException $e)
        {die("Error Getting Product Detail :".$e->getMessage());}
        $PD = $GetProductDetail->fetch(PDO::FETCH_ASSOC);
        $Return['Brand'] = $PD['brand'];
        $Return['ModelNumber'] = $PD['SKU'];
        $Return['Title'] = $PD['title'];
        $Return['ActualPrice'] = ($PD['pricingtype'] =='promo' && strtotime($PD['enddate']) >= time()?$PD['promoprice']:$PD['price']);
        $Return['MSRP'] = $PD['originalprice'];
        $Return['PictureUrl'] = $PD['picturelink'];
        $Return['Weight'] = $PD['weight'];
        $Return['DimensionalWeight'] = number_format($PD['height']*$PD['length']*$PD['width']/166,2);
        $Return['CalculatedWeight'] = ($Return['Weight']>=$Return['DimensionalWeight']?$Return['Weight']:$Return['DimensionalWeight']);
        return $Return;
    }
    public function __construct()
    {
//         Check if have an checkout already
        $Prepare = Checkout::$dbCon ->prepare("SELECT * FROM checkouts WHERE SessionId = :SessionId AND LastTouchTime >= :Time ORDER BY Id DESC");
        $Prepare -> bindValue(':SessionId',session_id());
        $Prepare -> bindValue(':Time',time()-self::$KeepCartFor);
        try{
            $Prepare -> execute();
            if($Prepare -> rowCount() != 0)
            {
                $Checkout = $Prepare->fetch(PDO::FETCH_ASSOC);
                $this->CheckoutId = $Checkout['Id'];
                $ThisVar = unserialize($Checkout['ObjectSerialized']);
                foreach($ThisVar as $Key => $Val)
                    $this->$Key = $Val;
                $this->CheckoutId = $Checkout['Id'];
            }
        }catch(PDOException $e)
        { die("Error Getting Checkout From Db: ".$e->getMessage()); }
    }
    public function __destruct()
    {
        if(is_null($this->CheckoutId))
        {// Insert Checkout In Mysql
            $CheckWhatIsNextId = Checkout::$dbCon->prepare("SELECT Id FROM checkouts ORDER BY Id DESC LIMIT 0,1");
            $CheckWhatIsNextId -> execute();
            $NextId = $CheckWhatIsNextId -> fetch(PDO::FETCH_ASSOC);
            $this->CheckoutId = $NextId['Id'];

            $InsertCheckout = Checkout::$dbCon->prepare("INSERT INTO checkouts (`SessionId`,`LastTouchTime`,`ObjectSerialized`) VALUES(:SessionId,:LastTouchTime,:ObjectSer);");
            $InsertCheckout -> bindValue(':SessionId',session_id());
            $InsertCheckout -> bindValue(':LastTouchTime',time());
            $InsertCheckout -> bindValue(':ObjectSer',serialize($this));
            try{
                $InsertCheckout -> execute();
            }catch(PDOException $e)
            {
                die("Error SavingCart In Db: ".$e->getMessage());
            }
        }
        else
        {
            $UpdateCheckout = Checkout::$dbCon->prepare("UPDATE `checkouts` SET `LastTouchTime` = :Time,ObjectSerialized = :Object WHERE `Id` = :Id;");
            $UpdateCheckout -> bindValue(':Time',time());
            $UpdateCheckout -> bindValue(':Object',serialize($this));
            $UpdateCheckout -> bindValue(':Id',$this->CheckoutId);
            try{
                $UpdateCheckout -> execute();
            }catch(PDOException $e)
            {
                die("Error SavingCart In Db: ".$e->getMessage());
            }
        }
    }
}

And this next class is simply a little class for my laziness of remembering the DSN of MySQL and port and all for PDO objects:

class dbCon extends PDO
{
    public function __construct($host,$port,$user,$pass,$dbName=null)
    {
        $pdo_options[PDO::ATTR_ERRMODE] = PDO::ERRMODE_EXCEPTION;
        $dsn = 'mysql:host='.$host.';port='.$port.';';
        if(!is_null($dbName))
            $dsn .='dbname='.$dbName;
        try{
            parent::__construct($dsn,$user,$pass,$pdo_options);
        }catch(PDOException $e){ die("Error Connecting To Database: ".$e->getMessage() ); }

    }
}

Technically on any page I can access the cart with only 2 line of code:

Checkout::$dbCon = new dbCon("127.0.0.1",3306,"UserName","Password","DatabaseName");
$Checkout = new Checkout();

Maybe there is some security issue that I'm not thinking about, or perhaps I should do something different. Let me know what you think.

\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

Database

  • Normalize your database! Don't keep a string of serialized objects in the database, that's extremely brittle and extremely not helpful in all but the most simple cases. Instead, you should have 3 tables (two of which you already have):
    • items - Table that holds information about items, includes name, price, etc.
      Key column: item_id.
    • carts (renamed from checkout) - Table that holds information about user's carts. There's a 1:1 relationship between carts and checkouts, a single cart can't be checked out more than once, and a single checkout doesn't apply to multiple carts. So it makes sense to have them in the same table. Has the cart's ID, the user's ID (in your case, SessionID) and extra information (not items) like LastTouchTime. Key column: cart_id
    • items-in-carts. Items and carts have a many to many relationship, also known as n:m ratio. One cart can have multiple items in it, and the same item can be in multiple carts. So we have a third table that ties the two together. All the table has is two columns: item_id, cart_id. Here's an example for a many-to-many architecture.

PHP

  • Naming convention - ClassNames should be CamelCaps, $variableNames and methodNames() should be lowercase camelCase. Don't mix caps with underscores (for example, Cart_AddItem should be just addItem, it clear we're talking about the cart here, isn't it?)
  • Use of static variables and methods - Don't. Static variables and methods are global, which means that by definition they make your application less stable, harder to test and maintain, and harder to read. Please don't.
  • __construct() function should always be first - The first thing I read about a class is how it's constructed.
  • Implicit dependencies - Your __construct() says it doesn't need any parameters, but that's a lie. It actually needs a database connection, and you're using a global to get it. That's what I mean by "don't use static variables". This is a better approach:

    public function __construct(DBCon $dbCon) {
    
  • Consistent spacing - Sometimes you use $var->method() and sometimes $var -> method() make up your mind, and stick with it. There are free tools that can do this job for you!

  • Naming of things - Why is it called a Checkout? What I see is a Cart, and that's how it should be called. If you can checkout a cart, you should have a method $cart->checkout(...). Your methods are redundantly long, Cart_Empty() can be empty(), Cart_AddItem() can be addItem().

  • Too much going on in once class - Your cart uses Items right? Why not make an Item class?

  • What's the purpose of the dbCon object? - Why are you placing another abstraction on top of PDO? What problem are you solving here? On 99% of the application (and I bet including yours), you only ever need a single database connection, so what's the point of wrapping it in a class that hinders readability (I know the PDO, but I see dbCon and I have no idea what it is, until I dive in and read the code).

All-in-all, I'd try to adopt a more OOP approach, create more class, increase the interaction between objects. Get rid of the globals and statics.

Good luck :)

\$\endgroup\$
6
  • \$\begingroup\$ Thanks for your comment/subjection. Id like to to say that the main purpuse of all this actualy was to make a full checkout system,. ( that is why I had Cart_addItem and so on. ) Also, The only reason why $dbCon is static is that when you serialize an object if there is an object that is a resource or PDO Object it can't be serialize, But I just got an idea while actuly writing this comment il try to use __sleep Again thanks alots for your time and your subjection. \$\endgroup\$ Commented Jun 12, 2014 at 11:32
  • \$\begingroup\$ Please don't serialize objects and save into the database. The results are unpredictable at best, and app-breaking at worse. Please, for your sake, don't. \$\endgroup\$ Commented Jun 12, 2014 at 12:51
  • \$\begingroup\$ I might sound ignorant but, Why? I have been using serialize for quite some time now and I never encounter something that is unpredictable, I am very willing to change the code to some normalized string, But only if I see some benefice. How can it be unpredictable? Do you have some fact that back what you are saying? \$\endgroup\$ Commented Jun 12, 2014 at 13:57
  • 2
    \$\begingroup\$ Well, here are some: You lose indexability (that hurts!), If you make changes to your table's interface or variable names, it invalidates all of your already stored serials. Performing a search by a specific thing in an object is impossible. You tightly couple the database with PHP (You cannot migrate to Java or a different server technology later). Is that enough reasoning, or would you like me to continue? \$\endgroup\$ Commented Jun 12, 2014 at 14:12
  • 1
    \$\begingroup\$ Sure thing ;) This is what this site is for. I actually put Stack Overflow on hold in favor of here to help people who actually want to help themselves. \$\endgroup\$ Commented Jun 12, 2014 at 14:57

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.