Задать вопрос
Ninazu
@Ninazu
php

Какие принципы SOLID здесь нарушены?

Решил я углубится в SOLID, и в частности сделать композицию интерфейсов, для решения следующей задачи.

Есть Box, в которую помещаются фигуры, часть из этих фигур реализуют интерфейс IShape, то есть могут быть залиты цветом, вторая часть реализует только IArea и не может быть залита цветом(линия, прозрачные, и т.д.). Нужно посчитать площадь всех залитых определённым цветом фигур в коробке.


И что-то мне подсказывает, что я нарушаю тут)))

class Box {

	/**@var IShape[] $shapes */
	private array $shapes = [];

	public function addShape(IArea $shape): self {
		$this->shapes[] = $shape;

		return $this;
	}

	public function getTotalAreaWithColor(int $color): float {
		$total = 0;

		foreach ($this->shapes as $shape) {
			if ($shape instanceof IFillColor && $shape->getFillColor() === $color) {
				$total += $shape->getArea();
			}
		}

		return $total;
	}

	public function getTotalArea(): float {
		$total = 0;

		foreach ($this->shapes as $shape) {
			$total += $shape->getArea();
		}

		return $total;
	}
}


interface IArea {

	public function getArea(): float;
}


interface IFillColor {

	public function setFillColor(string $rgb): self;

	public function getFillColor(): int;
}


interface IShape extends IArea, IFillColor {

}


trait TFillColor {

	protected int $fillColor = 0;

	public function setFillColor(string $rgb): self {
		$this->fillColor = (int)hexdec($rgb);

		return $this;
	}

	public function getFillColor(): int {
		return $this->fillColor;
	}
}
  • Вопрос задан
  • 1021 просмотр
6 комментариев
Подписаться 5 Простой 6 комментариев
Ответ пользователя Flying К ответам на вопрос (3)
@Flying
С моей точки зрения здесь в первую очередь нарушается S (принцип единственной ответственности). Box в целом не должен уметь что-то кроме хранения фигур. Как правильно заметил vgbege - из задачи не следует что в дальнейшем вариантов подсчёта каких-то метрик по фигурам с Box'е не станет больше, так что в первую очередь я бы выносил отдельно калькулятор. Также у вас нет обобщающего интерфейса для IArea и IShape из-за чего и возникает путаница в addShape.

В целом у меня получилось нечто подобное:
/**
 * Основной интерфейс для всех фигур которые можно хранить в Box'е
 */
interface IFigure
{

}

/**
 * Интерфейс для фигур которые могут обладать площадью
 */
interface IArea extends IFigure
{
    public function getArea(): float;
}

/**
 * Интерфейс для фигур которые могут обладать цветом заливки
 */
interface IFillColor
{
    public function setFillColor(string $color): self;

    public function getFillColor(): string;
}

/**
 * Обобщённый интерфейс для фигур, обладающих и цветом и площадью
 * По факту, думаю, он не имеет смысла, но нужен по условию задачи
 */
interface IShape extends IArea, IFillColor
{

}

/**
 * Коробка для хранения фигур
 */
class Box
{
    /**
     * @var IFigure[]
     */
    private $figures = [];

    /**
     * @param IFigure[] $figures
     */
    public function __construct(array $figures = [])
    {
        // Добавляем полученные через конструктор фигуры в объект
        // Используем addFigure() чтобы проконтролировать корректность типов
        array_walk($figures, [$this, 'addFigure']);
    }

    /**
     * @param IFigure $figure
     * @return $this
     */
    public function addFigure(IFigure $figure): self
    {
        $this->figures[] = $figure;
        return $this;
    }

    /**
     * @param IFigure $figure
     * @return bool
     */
    public function hasFigure(IFigure $figure): bool
    {
        return in_array($figure, $this->figures, true);
    }

    /**
     * @param IFigure $figure
     * @return $this
     */
    public function removeFigure(IFigure $figure): self
    {
        $this->figures = array_filter($this->figures, static function (IFigure $f) use ($figure) {
            return $f !== $figure;
        });
        return $this;
    }

    /**
     * @return IFigure[]
     */
    public function getFigures(): array
    {
        return $this->figures;
    }
}

/**
 * Интерфейс для калькуляторов фигур в Box'ах
 */
interface IFigureCalculator
{
    /**
     * @param Box $box
     * @param mixed ...$args
     * @return mixed
     */
    public function calculate(Box $box, ...$args);
}

/**
 * Пример калькулятора
 */
class AreaCalculator implements IFigureCalculator
{
    public function calculate(Box $box, ...$args): float
    {
        // Получаем список фигур которые обладают площадью
        $figures = array_filter($box->getFigures(), static function (IFigure $figure) {
            return $figure instanceof IArea;
        });
        // Получаем цвет из дополнительных аргументов калькулятора
        $color = array_shift($args);
        if ($color) {
            // У нас задан цвет, фильтруем фигуры по цвету
            $figures = array_filter($figures, static function (IFigure $figure) use ($color) {
                return $figure instanceof IFillColor && $figure->getFillColor() === $color;
            });
        }
        // Подсчитываем суммарную площадь
        return array_reduce($figures, static function (float $area, IArea $figure) {
            return $area + $figure->getArea();
        }, 0);
    }
}


В качестве альтернативы я рассматривал также вариант дать Box'у возможность самому считать метрики через создание у него метода:
public function calculate(IFigureCalculator $calculator, ...$args)

но потом решил что лучше не захламлять интерфейс чтобы не усложнять логику.
Ответ написан
7 комментариев
7 комментариев