Скажите как вам мой код?

Question

[PaffosONE]

Я только недавно начал писать на PHP, хочу узнать правильно ли я пишу ?
Пример кода:

<?php
require_once 'classes.php';

$product = new smartPhoneProduct('Xiomi 4X',12000,'Это Xiomi :D','DragonV',8,'64GB',"4 RAM");
echo $product->getProduct();
echo $product->getDiscount();

<?php
require_once 'noteBookProduct.php';

class Product
{
  public $name;
  protected $price;
  public $descriptions;

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

  public function getProduct()
  {
  echo "Имя: {$this->name}\nЦена: {$this->price} P\nОписания: {$this->descriptions}"; 
  }

}

<?php

class smartPhoneProduct extends Product 
{
	public    $discount = 15; // discount on product in (%).
	protected $cpu;
	protected $core;
	protected $memory;
	protected $ram;

	public function __construct($name,$price,$descriptions,$cpu,$core,$memory,$ram)
	{
		parent::__construct($name,$price,$descriptions);
		$this->cpu = $cpu;
		$this->core = $core;
		$this->memory = $memory;
		$this->ram = $ram;
	}

	public function getProduct()
	{
		$out = parent::getProduct();
		$out .= "\nПроцесор:{$this->cpu}\nКол-ядер:{$this->core}\nПамять:{$this->memory}\nОперативаня память:{$this->ram}";
		return $out;
	}

	public function getDiscount()
	{
		$discountOnProduct =  $this->price / 100 * $this->discount;
		echo "\nСкидка: " .$discountOnProduct. "P";
	}
}

Подскажите где мои ошибки, заранее спасибо.

Answer 1

[Максим Федоров]

require_once 'classes.php';

Перейти на Композер и неймспейсы

class smartPhoneProduct

Оставить Product, а тип товара вынести в свойство товара type или в category

Все, что касается модификаций — вынести в характеристики, например может подойтиEAV, то есть некая сущность Характеристика с product_id, name, value и таких быть может много у одного товара. Или иной способ работы, главное чтобы при добавлении сотен характеристик вам не пришлось бы сотнями файлов классы придумывать и всегда был один Product, ну или были др причины создавать детей, например простой товар и цифровой...
Альтернативы EAV обсуждались например тут: Альтернатива EAV, структура базы?

getProduct()

После исправления предыдущего шага данная штука product->getProduct() устранится, а сериализация должна быть в другом слое, товар не должен знать ничего про то, как отображать его, тк отображений может быть много (в каталоге, в письме, в отчетах, в корзине)

echo "\nСкидка: " .$discountOnProduct. "P";

echo — вывод, бизнес-объект не должен ничего знать про вывод, ответ сервера и прочее, должен быть просто return $this->description()
Тем более цена не должна идти в описании, а быть неким getPrice(), причем как правило это делают не у товара, а у вариантов/предложений товара, которые с товаром связаны 1 к 1

Скидка не должна быть частью товара, чтобы ее можно было навешивать по разным правилам/стратегиям и ситуациям, это скорее часть корзины/заказа, крч отдельная сущность со своим поведением (размер, проценты или абсолютный размер, когда применять ее).

это основное... к код-стайлу пока претензии не имеют смысла

Comments

[PaffosONE]

Спасибо большое Максим Федоров, постараюсь учесть все ошибки.

[Дмитрий]

PaffosONE, ну и скидка не должна быть публичной

[Максим Федоров]

Дмитрий, про скидку добавил в ответ :)

[DanielMcRon]

PaffosONE, и не забудьте поставить человеку галочку решения вопроса. Не зря же едалил свое время. А это всегда нужно и приятно

[Roman K]

Не стоит предлагать EAV как лучшее решение. У EAV огромное число минусов, о них стоит упоминать, имхо, всегда, когда используешь слово "EAV".

[Максим Федоров]

Roman Kitaev, спасибо большое, добавил ссылку на обсуждение