PHP OOP classes, abstract classes, interfaces and traits

[Borkg85]

Hi,

quite new to the PHP OOP principles, and am working on a task that requires implementing classes, abstract classes, setters/getters... So I have the products.class with the setters and functions and another product.abstract class with the getters and the reason for this is cause of a third process.class that should insert data into database. Is there a way for this to be done a bit more compact. Any tip or trick would be appreciated. Thanks...

Here is the code...

class Products extends Dbh
{

  private $sku;
  private $name;
  private $price;
  private $size;
  private $weight;
  private $height;
  private $length;
  private $width;


  public function __construct($id = 0, $sku = "", $name = "", $price = "", $size = "", $weight = "", $height = "", $length = "", $width = "")
  {
    $this->id = $id;
    $this->sku = $sku;
    $this->name = $name;
    $this->price = $price;
    $this->size = $size;
    $this->weight = $weight;
    $this->height = $height;
    $this->length = $length;
    $this->width = $width;
  }

  public function setSku($sku)
  {
      $this->sku = $sku;
  }

  public function setName($name)
  {
      $this->name = $name;
  }
  public function setPrice($price)
  {
      $this->price = $price;
  }

  public function setSize($size)
  {
      $this->size = $size;
  }
  
  public function setWeight($weight)
{
  $this->weight = $weight;
}

  public function setHeight($height)
  {
    $this->height = $height;
  }
  
  public function setLength($length)
  {
    $this->length = $length;
  }
  
  public function setWidth($width)
  {
    $this->width = $width;
  }


  public function setId($id)
  {
    $this->id = $id;
  }


  public function getBook() {
    try {
    $stmt = $this->connect()->prepare('SELECT * FROM skandi WHERE weight != 0');
    $stmt->execute();

    $results = $stmt->fetchAll();
    return $results;

    } catch (Exception $e) {
      return $e->getMessage();
    }

  }

  public function getDVD() {
    try {
    $stmt = $this->connect()->prepare('SELECT * FROM skandi WHERE size != 0');
    $stmt->execute();
    $results = $stmt->fetchAll();
    return $results;
    } catch (Exception $e) {
      return $e->getMessage();
    }

  }


  public function getFur() {
    try {
    $stmt = $this->connect()->prepare('SELECT * FROM skandi WHERE height != 0');
    $stmt->execute();
    $results = $stmt->fetchAll();
    return $results;
    } catch (Exception $e) {
      return $e->getMessage();
    }

  }

  public function insertData()
  {
    try {
      $stmt = $this->connect()->prepare("INSERT INTO skandi(sku, name, price, size, weight, height, length, width) 
           VALUES(?,?,?,?,?,?,?,?)");
      $stmt->execute([$this->sku, $this->name, $this->price, $this->size, $this->weight, $this->height, $this->length,  $this->width]);
    } catch (Exception $e) {
      return $e->getMessage();
    }
  }

  public function fetchAll()
  {
    try {
      $stmt = $this->connect()->prepare("SELECT * FROM skandi");
      $stmt->execute();
      return $stmt->fetchAll();
    } catch (Exception $e) {
      return $e->getMessage();
    }
  }

  public function delete()
  {
    try {
      $stmt = $this->connect()->prepare("DELETE FROM skandi WHERE id = ?");
      $stmt->execute([$this->id]);
      return $stmt->fetchAll();
    } catch (Exception $e) {
      return $e->getMessage();
    }
  }

  public function checkItemExists()
  {
    try {
      $stmt = $this->connect()->prepare("SELECT * FROM skandi WHERE sku = ?");
      $stmt->execute([$this->sku]);
      $result = $stmt->fetchAll(); 
      if(count($result) == 0) {
        return false;
        
    } 
        else {
        return true;
      }
      }catch (Exception $e) {
      return $e->getMessage();
    }
  }
}

the abstract class

abstract class Product
{

    private $sku;
    private $name;
    private $price;

    public function getSku()
    {
        return $this->sku;
    }

    public function getName()
    {
        return $this->name;
    }

    public function getPrice()
    {
        return $this->price;
    }
    
    abstract function getAttributes();


}

interface HaveSize {

    public function getSize();

}

trait Withsize {

    public function getSize()
    {
        $this->size;
    }

    function getAttributes() {
        return "$this->size KG";
      }
}

interface HaveWeight {

    public function getWeight();

    
}

trait WithWeight {

    public function getWeight()
    {
      $this->weight;
    }

    function getAttributes() {
        return "$this->weight KG";
      }

the process class..

 

$products = new Products;


if (isset($_POST['submit'])) {

    $products->setSku($_POST['sku']);
    $products->setName($_POST['name']);
    $products->setPrice($_POST['price']);
    $products->setSize(!empty($_POST['size'])) ? $_POST['size'] : NULL;
    $products->setWeight(!empty($_POST['weight'])) ? $_POST['weight'] : NULL;
    $products->setHeight(!empty($_POST['height'])) ? $_POST['height'] : NULL;
    $products->setLength(!empty($_POST['length'])) ? $_POST['length'] : NULL;
    $products->setWidth(!empty($_POST['width'])) ? $_POST['width'] : NULL;
    $products->insertData();

}

 

[requinix]

In no particular order, and not restricting my commentary to just OOP issues or your question of compactness,

- Inheritance represents an "is a" relationship. When you say "class X extends Y" then that means "X is a Y". It seems like Dbh is a database connection class. Would you say that Products "is a" database connection? I would not.
- PHP supports types for function arguments and return values. Use them. Types on class properties are generally a good idea too but they come with some nuances to consider.
- I don't think "price", "weight", "height", "length", or "width" are strings.
- It seems ridiculous that everything with a weight must be a book, or everything with a size is a DVD, or everything with a height is a fur.
- Your methods to get a book/DVD/fur have nothing to do with Products. You don't use the Products class for anything besides as a place to put these methods.
- Catching exceptions and then returning their error messages is... not good. If you are not going to actually handle the exception then do not use try/catch at all.
- You have interfaces and traits but aren't using them.
- If your Product's sku, name, and price are private and you don't provide any setter methods then how are those going to get values?
- It might make sense considering the weight/size/height problem, but since multiple traits cannot redefine the same method, your Withsize and WithWeight traits are mutually exclusive.
- What you're doing when you call the setter methods is not correct.

Having gone through that, I would say that you're not at the point where "compactness" should be a concern. In fact, compactness is almost never a thing that should be concerned at all. Focus more on whether your designs and architectures make logical sense: if they do then you will have compact code.

[maxxd]

I agree that especially at this point in learning OOP, don't concern yourself with "compactness" at all. This may be personal experience but in my career most of the 'compact' code I've come across is just entirely too clever. You can (it was easier prior to [I think] 8.1) nest ternary statements to such an extent that a complex if...elseif...else construct is on a single line. Compact? Absolutely. Readable? Not even slightly.

I'd even go so far as to say right now be overly verbose. Make absolutely sure you'll be able to follow not only your logic, but your thought process when you come back to this code in a few weeks. Document everything. The business logic of your app will be easy to forget in a week, and if you're cutting corners trying to write "compact" code you'll have no idea what the heck you were doing with a section of code, let alone why you were doing it.

After you get a good grip on OOP principles and how best to apply them to PHP, start thinking about conciseness of code; don't worry about how many lines it's taking, worry about how comfortable it is. Documentation is important, but there are certain situations where is slows down the reading and understanding of the code; in much the same way being too basic with the code and it's structure can slow reading and understanding almost as much as being too clever.

It's at this point that you'll recognize certain patterns and constructs that you won't necessarily need to document and that are generally readable. Just work on writing exactly the amount of code and documentation that needs to be written so you or someone else can pick the project back up in six months and not want to cry. You'll learn the middle ground that makes the code readable and maintainable (insomuch as you can - nobody's perfect and deadlines just ... well, suck).

[gizmola]

I want to throw in my 2 cents here:

• Why use an abstract class?
  • You have a number of concrete classes that will inherit attributes and behavior from this common ancestor.  The ancestor itself is not specific enough to be an actual thing.
    • In your case you should decide -- is Product "abstract" for your use?  It doesn't seem to be, but by no means should you have a concrete class named Products!  If you want to use plurals, great, but most often people will use singular naming.  It seems that you did this Product vs Products thing just to get around the naming issue, which should tell you that you are on the wrong track here.
    • In Products, your constructor just sets a lot of of attributes, all of which could easily be part of the Abstract Product class.  Move that into the Abstract class.
      • One obvious thing missing is the idea of what "type" of Product an individual Item is.   Your specific getFur, getDVD etc, are just completely off base and shouldn't exist.
        • The database should have a type or item_type column in it that describes the type of item.  This would typically be constrained by a table which has a row for each possible item type, used as a foreign key for your products or items table.
  • What's an alternative approach here?
    • MVC would tell you that you could use a "Model" class.  Generically a model class knows how to map data from a database table into attributes.  We know from your code that table is named "skandi"  So taking a step back, how about thinking about what you might need to facilitate the data from a table.  What might you have generically?

<?php

abstract class TableModel {
    private $databaseConnection;
    protected $database;
    protected $table;
    protected $primaryKeys = [];
    protected $attributes = [];
    private $data = [];
    
    protected function getDatabase() {
        return $this->database;
    }
    
    protected function getTable() {
        return $this->table;
    }
    
    protected function getPrimaryKeys() {
        return $this->primaryKeys;
    }
    
    protected function setAttributes(array $attributes) {
        $this->attributes = $attributes;
    }
    
    public function getAttributes() {
        return array_merge($this->primaryKeys, $this->attributes);
    }
    
    public function setDatabaseConnection($databaseConnection) {
        $this->databaseConnection = $databaseConnection;
    }
    
    protected function getDatabaseConnection() {
        if (!$this->databaseConnection) {
            throw new \Exception('Database Connection Uninitialized');
        }
        return $this->getDatabaseConnection;
    }
    
    protected function getFullTableName() {
        return '`' . $this->getDatabase() . '.' . $this->getTable() . '`';
    }
    
    public function getAllRowsSQL() {
        // Just an example to show the possibilities.
        return 'SELECT * FROM ' . $this->getFullTableName();
    }
    
}

class Product extends TableModel {
    protected $database = 'yourDb';
    protected $table = 'skandi';
    protected $primaryKeys = ['id'];
    protected $attributes = ['sku', 'name'];
}


$product = new Product();
$conn = 'SomeDbConnection';
$product->setDatabaseConnection($conn);


echo $product->getAllRowsSQL() . PHP_EOL;

echo implode(',', $product->getAttributes()) . PHP_EOL;

 

You most likely wouldn't have a getAllRowsSQL method, but I provided it just to give you an idea of where you might go with this.  

Since TableModel is generic for any type of table, you can concentrate on how to generically query data from any table and load it into the $data attribute.  You could also consider things like insert/update and delete.  

All the other attributes you have which are specific to the skandi table, might lead you in the direction of an Interface or possibly an interface with a set of common "Product" traits.  

I only mention this because you seem to be on the path of needing to explore those features.   In general, interfaces allow for passing a de-coupled object into a class as a parameter.  Rather than have the specific insert/update/delete routines in a TableModel, you might instead want your abstract TableModel class to implement an interface that a separate database management class might want to call instead.   getFullTableName() is an example of a method that might be helpful in a database management class, where you would pass a Product object into and have it do the specific SQL calls, using things like the table name, keys and attributes.  

[Borkg85]

Hi, sorry for my late input, I am also trying to do the same task as a simple php rest api using ajax and javascript.

Thank you all for the helpful advices,  since it really gives me an insight  trying to make sense of the OOP principles, classes, abstract, setters, getters, inheritance, polymorphism, and this is a test task for a job that I applied that needs to be done with OOP.

The interfaces and traits were connected to 3 different classes, that should have get all the values and list them in an index.php table.

However with this current setup that won't work, since the functions and certain properties don't make sense and are just badly written. 

I definitively will be trying to make this work in the coming days...

Edited October 22, 2022 by Borkg85

[Borkg85]

11 hours ago, Borkg85 said:

Hi, sorry for my late input, I am also trying to do the same task as a simple php rest api using ajax and javascript.

Thank you all for the helpful advices,  since it really gives me an insight  trying to make sense of the OOP principles, classes, abstract, setters, getters, inheritance, polymorphism, and this is a test task for a job that I applied that needs to be done with OOP.

The interfaces and traits were connected to 3 different classes, that should have get all the values and list them in an index.php table.

However with this current setup that won't work, since the functions and certain properties don't make sense and are just badly written. 

I definitively will be trying to make this work in the coming days...

So I've managed to do some changes and I will post the code... 

post.class for now with only the read option... 

class Post 

{
  //DB 
  private $conn;
  private $table = 'skandi';

  //Properties 


  public function __construct($db) {
    $this->conn = $db;
  }
  

  public function read()
  {
    $query = 'SELECT 
    p.id as id, 
    p.sku,
    p.name, 
    p.price, 
    p.productType,
    p.size, 
    p.weight, 
    p.height, 
    p.length, 
    p.width FROM '. $this->table .' p  ORDER BY p.id DESC'; 

  $stmt = $this->conn->prepare($query);
  $stmt->execute();

  return $stmt;

  }

product.class, I would like to use the productType == 'dvd, book or furniture' to display the values only when there is one in the product show class, however I cannot be using if/else conditionals...

I could do separate methods like getDVD(), getBook(), instead of the getproductType(), might be more manageable...  

abstract class Product
{

  private $sku;
  private $name;
  protected $price;
  protected $productType;
  

  public function __construct($sku, $name, $price)
  {
    
    $this->sku = $sku;
    $this->name = $name;
    $this->price = $price;
    // $this->productType = $productType;
   
  }

  public function setSku($sku)
  {
      $this->sku = $sku;
  }

  public function getSku()
  {
      $this->sku;
  }

  public function setName($name)
  {
      $this->name = $name;
  }

  public function getName()
  {
     $this->name;
  }

  public function setPrice($price)
  {
      $this->price = $price;
  }

  public function getPrice()
  {
    return  '$this->price $';
  }
  
  public function getproductType(){
    
  }


}

interface HaveSize {

  
    public function getSize();

}

trait Withsize {

  public function setSize($size)
  {
    $this->size = $size;
  }

    public function getSize()
    {
        $this->size;
    }

    function getproductType() {
        return "$this->size KG";
      }
}

interface HaveWeight {

    public function getWeight();

    
}

trait WithWeight {
  public function setWeight($weight)
  {
    $this->weight = $weight;
  }

    public function getWeight()
    {
      $this->weight;
    }

    function getproductType() {
        return "$this->weight KG";
      }

}

interface HaveFurniture {

    public function getHeight();
    public function getLength();
    public function getWidth();

    
}

trait WithFurniture {

  public function setHeight($height)
  {
    $this->height = $height;
  }
  
  public function setLength($length)
  {
    $this->length = $length;
  }
  
  public function setWidth($width)
  {
    $this->width = $width;
  }

    public function getHeight()
    {
      $this->height;
    }
    
    public function getLength()
    {
      $this->length;
    }
    
    public function getWidth()
    {
      $this->width;
    }

    function getproductType() {
        return "$this->height CM x $this->length CM x $this->width CM";
      }

}

one of the producttype classes...

class DVD extends Product implements HaveSize {

    private $size;

    public function __construct($sku, $name, $price, $size) {

        parent::__construct($sku, $name, $price);

        $this->size = $size;
    }
    
    use Withsize;


}

the productshow.class

class ProductShow extends Post

{
    public function showAllProducts(){

        $allProducts = array();

        $stmt = $this->read();
        foreach($stmt as $result){
            $book = new Book($result['sku'], $result['name'], $result['price'], $result['weight']);
            array_push($allProducts, $book);
        }

        $stmt = $this->read();
        foreach($stmt as $result){
            $dvd = new Dvd($result['sku'], $result['name'], $result['price'], $result['size']);
            array_push($allProducts, $dvd);
        }
 
        $stmt = $this->read();
        foreach($stmt as $result){
            $furniture = new Fur($result['sku'], $result['name'], $result['price'], 
            $result['height'], $result['width'], $result['length']);
            array_push($allProducts, $furniture);
        }

        foreach($allProducts as $product){
            echo "<table>
                    <tbody>
                    <tr class='content'>
                    <th> <input type='checkbox' name='checkbox[]' > </th>
                    <td>" .$product->getSku(). "</td>
                    <td>" .$product->getName(). "</td>
                    <td>" .$product->getPrice(). "</td>
                    <td>" .$product->getproductType(). "</td>
                    <td>" .$product->getproductType(). "</td>
                    <td>" .$product->getproductType(). "</td>
                    </tr>
                    </tbody>
                    </table>";
        }
    }
}

and the result that I'm getting is presented in the image...

 

[Borkg85]

Hi, 

So ignoring my previous post, since I cannot find an option to delete it, however I did managed to fix the issues and make it work. I then deleted it all and am currently trying to make the tablemodel work, but I cannot seem to do so with my limited knowledge. 

Should I be doing the read/insert/delete methods in the tablemodel abstract class, like it was shown in the @gizmola example?

abstract class Product
{

    private $conn;
    protected $table;
    protected $primaryKeys = [];
    protected $uniqueKeys = [];
    protected $attributes = [];
    private $data = [];


    protected function getTable() {
        return $this->table;
    }

    protected function getPrimaryKeys() {
        return $this->primaryKeys;
    }

    protected function setAttributes(array $attributes) {
        $this->attributes = $attributes;
    }

    public function getAttributes() {
        return array_merge($this->primaryKeys, $this->attributes);
    }

    public function setDatabaseConnection($conn) {
        $this->conn = $conn;
    }

    protected function getDatabaseConnection() {
        if (!$this->conn) {
            throw new \Exception('Database Connection Uninitialized');
        }
        return $this->getDatabaseConnection;
    }

}

or maybe something like this, only thing to note is that the productType is a column with type dvd, book, furniture with which I should connect the size, weight...? Maybe this would be one way to go about that issue and then making insert methods for dvd, book, furniture classes...

public function productType(array $array)
    {
        if (isset($array)) {
            if (array_key_exists('size', $array)) {
                $this->productType = 'dvd';
            } elseif (array_key_exists('weight', $array)) {
                $this->productType = 'book';
            } elseif (array_key_exists('height', $array)) {
                $this->productType = 'furniture';
            }
        }
    }

class Post extends Product {

    private $conn;
    protected $table = 'skandi';
    protected $primaryKeys = ['id'];
    protected $uniqueKeys = ['sku', 'name'];
    protected $attributes = ['price',
        'productType', 'size', 'weight', 'height', 'length', 'width'];


    public function read(): array
    {

        $query =  'SELECT * FROM '. $this->getTable() .' ORDER BY id DESC' ;
        $stmt = $this->conn->prepare($query);
//        $stmt->execute();
        $data = [];
        while($result = $stmt->fetchAll(PDO::FETCH_ASSOC)){
            $product=new Post();
            $product->setAttributes($result);

            $data[]=$product;

    }
        return $data;


    }

this is the display index page, which maybe could be done by echoing this into the read method...

  $products = new Post($db);


    foreach($products->read() as $product) {

    echo "<table>";
    echo        "<tbody>";
    echo         "<tr class='content'>";
    echo          "<th> <input type='checkbox' name='checkbox[]'> </th>";
    echo            "<td style='visibility: hidden'></td>";
    echo            "<td>" . implode(',', $products->getAttributes())."</td>";


    echo           "</tr>";
    echo        "</tbody>";
    echo      "</table>";

   }
      ?>

Any advice, including some tutorials would be great as I am struggling to complete this task.

Edited November 8, 2022 by Borkg85