Product and subclasses
Your code is old fashioned and easy to misuse due to the problems you have with OOP. Consider this, which you're letting me do with your code:
$dvd = new DVD("12345", "Your Name", "15 dollars", "Romance", "35MB");
echo($dvd->formatDescription("Not the description"));
This shows that it's easy to make a lot of mistakes. I'm echoing out something that's not the description (an OOP problem), and several variables with an incorrect format (which can be fixed via modern PHP).
Here's a quick rewrite of the Product and DVD classes:
abstract class Product
{
public function __construct(
private readonly string $sku,
public readonly string $name,
public readonly int $price,
public readonly string $type,
protected readonly int|array $description
){}
public function formatSKU(): string
{
return strtoupper(substr($this->type, 0, 4)) . str_pad($this->sku, 4, "0", STR_PAD_LEFT);
}
abstract protected function formatDescription(): string;
}
class DVD extends Product
{
public function formatDescription(): string
{
return "Size: " . $this->description . "MB";
}
}
// Test it:
$dvd = new DVD("12345", "Your Name", 1500, "Romance", 35);
echo($dvd->formatSKU() . ' ' . $dvd->formatDescription());
Some highlights:
- Used
$this->variableName
instead of redundantly and possibly mistakenly passing in the variable from outside the class
- Constructor promotion (PHP 8.0)
- Readonly properties (PHP 8.1), which replace your getters while not allowing accidental overwrites. (You don't ever re-set the variable so why would you need setters?)
- Type declarations and return declarations (not even that new)
- Renamed some variables to be more clear ($description) or less redundant (the Product's prod variables).
The $description being the union type (PHP 8.0) "int|array" shows how weird your design is for this particular variable (since it is an array of two ints for the furniture class and an int everywhere else), so that's something I'd also refactor.
I'm also not so sure about the $type... can it really be any string? Maybe you could use an enum (or several) from PHP 8.1 to lock that down to the only types that should exist.
Main file
What really sticks out to me here is the lack of validation. You made an attempt to ensure that all the POST variables were set, but completely forgot $class, or to check that any of the variables contained what you wanted (e.g. a positive int for the price, in cents — which would avoid the possibility of floating point errors). You're assuming that your frontend is going to pull all the weight, but it can't: a user can submit a POST request with whatever they want. Especially after the rewrite you need to validate, since the outcome otherwise will be a server error. Especially make sure that $class
is one of the classes you want (which I would use match
from PHP 8 for).
Also $api->addProduct($sku, $name, $price, $type, $description);
should probably accept a Product instead, if you have any control over that.