Какие принципы 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;
	}
}
  • Вопрос задан
  • 975 просмотров
Пригласить эксперта
Ответы на вопрос 3
@vgbege
сразу бросается в глаза O+I я бы сказал
представь, что после нужно будет считать площадь только маленьких фигур (area < 3) или добавится интерфейс с текстурой и нужно будет считать только пушистые.

не, формально принцип O пока еще не нарушен, и класс Box можно расширять, добавляя в потомках getTotalAreaSmall и getTotalAreaFluffy. но Box уже сейчас выглядит не просто как Box (который просто хранит фигуры), а как BoxThatCountsColored, а его потомки будут называться BoxThatCountsColoredAndSmallAndFluffy например :)
Ответ написан
@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)

но потом решил что лучше не захламлять интерфейс чтобы не усложнять логику.
Ответ написан
Ninazu
@Ninazu Автор вопроса
Сделал через Visitor, по моему получилось неплохо.

<?php

interface IVisitor {

	public function getResult();

	public function visit(IShape $element): void;
}

interface IShape {

	public function accept(IVisitor $visitor): void;
}

interface IArea extends IShape {

	public function getArea(): float;
}

interface IFillColor extends IShape {

	public function setFillColor(string $rgb);

	public function getFillColor(): int;
}

trait TFillColor {

	protected $fillColor = 0;

	public function getFillColor(): int {
		return $this->fillColor;
	}

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

class Line implements IArea {

	protected $length;

	public function __construct(float $length) {
		$this->length = $length;
	}

	public function getArea(): float {
		return $this->length;
	}

	public function accept(IVisitor $visitor): void {
		$visitor->visit($this);
	}
}

class Rectangle implements IShape, IFillColor {

	use TFillColor;

	protected $width;

	protected $height;

	public function __construct(float $width, float $height) {
		$this->width = $width;
		$this->height = $height;
	}

	public function getArea(): float {
		return $this->width * $this->height;
	}

	public function accept(IVisitor $visitor): void {
		$visitor->visit($this);
	}
}

class Box {

	/**@var IShape[] $shapes */
	protected $shapes = [];

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

		return $this;
	}

	public function executeVisitor(IVisitor $visitor) {
		foreach ($this->shapes as $shape) {
			$shape->accept($visitor);
		}

		return $visitor->getResult();
	}
}

class TotalAreaWithColor implements IVisitor {

	protected $total = 0;

	protected $color;

	public function __construct(int $color) {
		$this->color = $color;
	}

	public function visit(IShape $element): void {
		if ($element instanceof IFillColor && $element->getFillColor() === $this->color) {
			$this->total += $element->getArea();
		}
	}

	public function getResult() {
		return $this->total;
	}
}

$rect  = new Rectangle(4, 10);
$rect->setFillColor("FF0000");

$line = new Line(10);

echo (new Box())
	->addShape($rect)
	->addShape($line)
	->executeVisitor(new TotalAreaWithColor(hexdec("FF0000")));
Ответ написан
Ваш ответ на вопрос

Войдите, чтобы написать ответ

Войти через центр авторизации
Похожие вопросы