Product search script Object Oriented PHP

I have made a search script in OO PHP that searches for products based on the inserted criteria, selects them and checks whether a sale is on, if it's in stock and other features.

I am new to OOP so I would like to get your feedback on it and whether I am doing it correctly.

The whole script works but I need to know if I'm doing OK.

Many thanks.

Here is my search class (class.search.php)

db = $db_connection; //stores provided db connection in variable
    }

    function search($searchContent, $max){

        $searchContent = "%".$searchContent."%";

    if ($max == 0){
      $searchQuery = "SELECT * FROM products WHERE title LIKE ?";
    }else{
      $searchQuery = "SELECT * FROM products WHERE title LIKE ? LIMIT $max";
    }

    try{
       $stmt = $this->db->prepare($searchQuery); //querys db for products that contain the query in the title
       $stmt->bindParam(1, $searchContent);
       $stmt->execute();
    }catch(PDOException $e) {
       echo $e->getMessage();
    }    

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

    if ($stmt->rowCount() != 0){ ///checks if there were any results

      foreach ($result as $row) { //stores all info/variables in an array

        $this->productCount++; //adds to the number of products

        $this->productTitle[] = $row["title"];
        $this->productImage[] = $row["image"];
        $this->productPrice[] = $row["price"];
        $this->productSalePrice[] = $row["salePrice"];
        $this->productStock[] = $row['quantity'];
        $this->productId[] = $row['id'];

      }

    }

    }

  function getPriceToDisplay($productSalePrice, $regularPrice){

    if ($productSalePrice != 0){

      return "£".$regularPrice." £".$productSalePrice;

    }else{

        return "£".$regularPrice;

    }

  }

  function getStockClass($stock){ //method to get the class for the css

      if ($stock 

Here is the search script (searchStore.php)

```
search($searchContent, 0);

if (strLen($se

Security: SQl Injection

You use prepared statements, which is great. But you still put variables directly into SQL queries, which makes you vulnerable to SQL injection via $max:

$searchQuery = "SELECT * FROM products WHERE title LIKE ? LIMIT $max";

At a minimum you would need to check if $max is an integer. But really, the only proper solution is to use prepared statements for all variables.

Security: XSS

You are inserting data from the database into HTML without encoding or sanitation, which will lead to XSS by anyone who can add products. This may or may not be acceptable at the moment, but you should also think about future requirements (maybe not everyone who can add products should have full admin rights?).

Structure/OOP


I am new to OOP so I would like to get your feedback on it and whether I am doing it correctly.

Not really. Ideally, a class should be responsible for one thing. But your class does quite a lot. As its name suggest, it searches, but it also builds HTML, transforms error codes to error messages, and checks stock values.

First of all, it would be a good idea to separate all code that builds HTML or other output in one place. This will make it a lot easier to modify it later on. Ideally, you would use a templating engine for this.

The previous point should get rid of your getPriceToDisplay method, which really doesn't belong in this class. But you should also move getPriceToDisplay, getStockStatus, and searchError somewhere else. getStockClass might belong in a Stock class, and getStockStatus and searchError in some component that transfers labels to messages (having this all in one place makes adaptation easier).

You also have quite a lot of fields in your search class, which are also rather confusing (having arrays of the different properties of items instead of an array of items). You could easily get rid of the fields by adding a Product class. You also don't really need productCount.

In general, you might want to look into MVC. You don't have to follow it 100%, but it's a good first approach.

Style

Your indentation is off, and your formatting is not consistent (spacing, vertical whitespace, etc), making your code unnecessarily hard to read.

You can use any IDE to fix most formatting issues automatically.

Naming

Your variable names are generally quite good. One small thing:

• be consistent with your variable names. Either it's camelCase or snake_case, not both.

Your function names could probably use some improvement. Eg:

• searchError: this doesn't search an error, it transforms a label into an error message. It might be called labelToErrorMessage.

• getStockClass: it isn't obvious that this is about CSS at all. This should be expressed via the function name.

• getPriceToDisplay: getPriceToHTML may be more explicit.

But you can see that most of these renames are not necessary or would be obvious if the functions were moved to a better context.

Comments

Your comments are helpful for people learning PHP, but if this isn't code for a tutorial, you should remove them. Inline comments shouldn't duplicate what the code already told us, but explain why the code is written the way it is.

Your function comments would be better as PHPDoc comments. Also, some of them could be a bit more in-depth (eg searchError: how does it "deal with errors"?, etc).

Misc

• don't echo exception messages inside classes, it makes them difficult to reuse. Throw them, and let the calling code handle it.

StackExchange Code Review Q#136737, answer score: 7