2
\$\begingroup\$

I have made a shopping basket script that allows the user to add and remove items from the basket, and edit quantity of items that are in the basket.

I am new to OOP, and I would like to get your feedback on it about whether I am doing it well. The whole script works but I need to know if it could be improved or it is fine.

<?php

Class Item{

  public $db, $item_id, $result, $image, $title, $description, $stock, $price, $salePrice, $isFreeDelivery, $deliveryPrice;

  function __construct($db_connection, $item_id){
    $this->db = $db_connection;
    $this->item_id = $item_id;

    try{

      $stmt = $this->db->prepare("SELECT * FROM products WHERE id = :item_id LIMIT 1"); 
      $stmt->bindParam(':item_id', $this->item_id);
      $stmt->execute();

    }catch(PDOException $e) {

      echo $e->getMessage();

    }    

    $this->result = $stmt->fetchAll(PDO::FETCH_ASSOC);

    $this->image = $this->getImage();
    $this->title = $this->getTitle();
    $this->description = $this->getDescription();
    $this->stock = $this->getStock();
    $this->price = $this->getPrice();
    $this->salePrice = $this->getSalePrice();
    $this->isFreeDelivery = $this->isFreeDelivery();
    $this->deliveryPrice = $this->getDeliveryPrice();


  }

  function getImage(){

    foreach($this->result as $row){
      return $row["image"];
    }

  }

  function getTitle(){

    foreach($this->result as $row){
      return $row["title"];
    }

  }

  function getDescription(){

    foreach($this->result as $row){
      return $row["description"];
    }

  }

  function getStock(){

    foreach($this->result as $row){
      return $row["quantity"];
    }

  }

  function getPrice(){

    foreach($this->result as $row){
      return $row["price"];
    }

  }

  function getSalePrice(){

    foreach($this->result as $row){
      return $row["salePrice"];
    }

  }

  function getAmountSaved(){
    return "&pound;".($this->getPrice()-$this->getSalePrice());
  }

  function getPercentageOff(){
    return round($this->getSalePrice() / $this->getPrice() * 100)."%";
  }

  function isFreeDelivery(){

    foreach($this->result as $row){
      return $row["freeDelivery"];
    }

  }

  function getDeliveryPrice(){

    foreach($this->result as $row){
      return $row["deliveryPrice"];
    }

  }

}

?>

class.basket.php

<?php

Class Basket{

  public $db, $user_id, $item_id = array(), $itemsInBasket = 0;

  function __construct($db_connection, $user_id){
      $this->db = $db_connection;
      $this->user_id = $user_id;
  }


  function basketCount(){

    try{

      $stmt = $this->db->prepare("SELECT * FROM tbl_users WHERE userid = :user_id LIMIT 1"); 
      $stmt->bindParam(':user_id', $this->user_id);
      $stmt->execute();

    }catch(PDOException $e) {

      echo $e->getMessage();

    }    

    $result = $stmt->fetchAll(PDO::FETCH_ASSOC);

    foreach($result as $row){
      return $row["itemsBasket"];
    }

  }

  function countItemNo($item_id){ //returns quantity in basket of a single product

    try{

      $stmt = $this->db->prepare("SELECT * FROM basket WHERE item_id = :item_id AND user_id = :user_id LIMIT 1"); 
      $stmt->bindParam(':item_id', $item_id);
      $stmt->bindParam(':user_id', $this->user_id);
      $stmt->execute();

    }catch(PDOException $e) {

      echo $e->getMessage();

    }    

    $result = $stmt->fetchAll(PDO::FETCH_ASSOC);

    foreach($result as $row){
      return $row["quantity"];
    }

  }

  function isInBasket($item_id){

    try{

      $stmt = $this->db->prepare("SELECT * FROM basket WHERE item_id = :item_id AND user_id = :user_id LIMIT 1"); 
      $stmt->bindParam(':item_id', $item_id);
      $stmt->bindParam(':user_id', $this->user_id);
      $stmt->execute();

    }catch(PDOException $e) {

      echo $e->getMessage();

    }   



    $numRows = $stmt->rowCount();

    if ($numRows == 1){
      return true;
    }else{
      return false;
    }


  }

  function addToBasket($item_id, $quantity){

    //add to user basket count and to basket table
    //add number of product to add
    //check if in basket to increase

    if ($this->isInBasket($item_id) == false){

      try{

        $stmt = $this->db->prepare("INSERT INTO `storify`.`basket` (`id`, `item_id`, `quantity`, `user_id`) VALUES (NULL, :item_id, :quantity, :user_id)"); 
        $stmt->bindParam(':item_id', $item_id);
        $stmt->bindParam(':quantity', $quantity);
        $stmt->bindParam(':user_id', $this->user_id);
        $stmt->execute();

      }catch(PDOException $e) {

        echo $e->getMessage();

      }

    }else{


      try{

        $stmt = $this->db->prepare("UPDATE `storify`.`basket` SET quantity = quantity + :quantity WHERE item_id = :item_id AND user_id = :user_id LIMIT 1"); 
        $stmt->bindParam(':item_id', $item_id);
        $stmt->bindParam(':quantity', $quantity);
        $stmt->bindParam(':user_id', $this->user_id);
        $stmt->execute();

      }catch(PDOException $e) {

        echo $e->getMessage();

      }

    }


    try{

      $stmt = $this->db->prepare("UPDATE `storify`.`tbl_users` SET itemsBasket = itemsBasket + :quantity WHERE userid = :user_id"); 
      $stmt->bindParam(':quantity', $quantity);
      $stmt->bindParam(':user_id', $this->user_id);
      $stmt->execute();

    }catch(PDOException $e) {

      echo $e->getMessage();

    }      


  }

  function removeFromBasket($item_id){ //remove all of an item from basket

    //get quantity in basket
    //decrease that quantity in user table
    //remove item basket row from basket table

    $quantityInBasket = $this->countItemNo($item_id);


    try{

      $stmt = $this->db->prepare("UPDATE `storify`.`tbl_users` SET itemsBasket = itemsBasket - :quantity WHERE userid = :user_id"); 
      $stmt->bindParam(':quantity', $quantityInBasket);
      $stmt->bindParam(':user_id', $this->user_id);
      $stmt->execute();

    }catch(PDOException $e) {

      echo $e->getMessage();

    } 

    try{

      $stmt = $this->db->prepare("DELETE FROM basket WHERE item_id = :item_id AND user_id = :user_id"); 
      $stmt->bindParam(':item_id', $item_id);
      $stmt->bindParam(':user_id', $this->user_id);
      $stmt->execute();

    }catch(PDOException $e) {

      echo $e->getMessage();

    } 


  }

  function editBasketQuantity($item_id, $new_quantity){

    //get quanity in basket
    //check that there are still at least one left - if not, run removeFromBasket($item_id);

    try{

      $stmt = $this->db->prepare("UPDATE basket SET quantity = :new_quantity WHERE item_id = :item_id AND user_id = :user_id"); 
      $stmt->bindParam(':new_quantity', $new_quantity);
      $stmt->bindParam(':user_id', $this->user_id);
      $stmt->bindParam(':item_id', $item_id);
      $stmt->execute();

    }catch(PDOException $e) {

      echo $e->getMessage();

    } 



  }



  function selectAllFromBasket(){

    try{

      $stmt = $this->db->prepare("SELECT * FROM basket WHERE user_id = :user_id");
      $stmt->bindParam(':user_id', $this->user_id);
      $stmt->execute();

    }catch(PDOException $e) {

      echo $e->getMessage();

    }

    if ($stmt->rowCount() != 0){

      $result = $stmt->fetchAll(PDO::FETCH_ASSOC);

      foreach ($result as $row){

        $this->itemsInBasket++;

        $this->item_id[] = $row['item_id'];

      }

    }else{
      echo "<p style='font-family: arial;'>No items in basket</p>";
    }




  }




}

?>

And finally here is basket.php

<?php

include_once "includes/header.php";
require_once "classes/class.db.php";
require_once "classes/class.item.php";
require_once "classes/class.user.php";
require_once "classes/class.basket.php";


if (User::logged_in() != false){

    $basket = new Basket($db, $_SESSION['user_id']);

    //prints out containers and title
    echo "
    <div id='itemInfoNegative' style='width: 610px; padding: 5px;'></div>
    <div id='itemInfoPositive' style='width: 610px; padding: 5px;'></div>
    <div id='inBasketContainer'>
    <h3>In Basket</h3>";

    $basket->selectAllFromBasket();

    for ($i = 0; $i < $basket->itemsInBasket; $i++) { //loops through items in basket

        $item_id = $basket->item_id[$i];
        $item = new Item($db, $item_id);

        echo "<div class='item'>
        <img src='".$item->getImage()."'/>
        <p class='itemTitle'><a href='item.php?item_id=$item_id'>".$item->getTitle()."</a></p><br>";

        if ($item->getStock() >= $basket->countItemNo($item_id)){
            echo "<p class='inBasketInStock'>In Stock</p>";
        }else if ($item->getStock() == 0){
            echo "<p class='inBasketLowStock'>Out of Stock</p>";
        }else{
            echo "<p class='inBasketLowStock'>Only ".$item->getStock()." left in stock</p>";
        }

        echo "
        <form method='post' class='deleteFromBasketForm'>
        <input type='hidden' name='item_id' value='$item_id' />
        <input type='submit' class='deleteFromBasket_$item_id' class='deleteFromBasket' value='Remove'/>
        </form>
        <br>
        <p class='deleteFromBasketLink' onClick='removeFromBasketClick($item_id)'>Remove</p>

        <select class='selectQuantity' onchange='getval(this);'>";

        for ($x = 1; $x <= $item->getStock(); $x++){

            if ($x == $basket->countItemNo($item_id)){
                echo "<option selected='selected' value='$x'>$x</option>";
            }else{
                echo "<option value='$x'>$x</option>";
            }

        }

        echo "</select>
        </div>"; //ends .item container

    } 

    echo "</div>"; //ends .inBasketContainer container

}

?>
\$\endgroup\$
1
\$\begingroup\$

In the item class you don't really need functions to get each part of the item unless you intend on making all of those functions public so other classes/files may use them.

You use a foreach loop in each of the functions but the select in the construct method will only ever return 1 result (LIMIT 1 in the query). No need for looping.

$this->result = $stmt->fetchAll(PDO::FETCH_ASSOC);
$row = $this->result[0];  // first and only record in the array

$this->image = $row['image'];
$this->title = $row['title'];

etc ... notice also that I used single quotes around image instead of double quotes. There is no reason to have PHP determine whether or not that is a variable or a constant. In this case, and for the column names for the other items, it is always going to be a constant.

You do something similar in your basket class and that could be updated as shown above (unless there are methods there that you want available to other classes/files).

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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