Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Шмаков Данил #192

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Open

Шмаков Данил #192

wants to merge 11 commits into from

Conversation

Reltig
Copy link

@Reltig Reltig commented Jan 23, 2024

No description provided.

TagCloud/TagCloudGenerator.cs Outdated Show resolved Hide resolved
TagCloud/TagCloudGenerator.cs Outdated Show resolved Hide resolved
TagCloud/CircularCloudLayouter.cs Outdated Show resolved Hide resolved
);
}

public static SpiralCloudShaper Create(Point center, double coefficient = 0.1, double deltaAngle = 0.1)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Немного контекста о том, когда и как обычно создаются такие методы:

  1. Модели - простые контейнеры для свойств. В твоем случае класс Options, где ты как раз и применил этот подход со статическим созданием. Из общих примеров можно взять FluentResults - там результат можно создавать как Result.Fail() и Result.Ok() (за точность методов не ручаюсь, просто показываю подход)
  2. Билдеры сложных моделей - Builder.Build() - обычный паттерн строитель, когда тебе нужно собрать сложную модель
  3. Фабрики моделей и зависимостей - паттерн фабрики (в разных вариациях) - обычно используется как раз для создания зависимостей для отдачи в контейнере

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

TagCloud/CloudDrawers/CloudDrawer.cs Outdated Show resolved Hide resolved
TagCloudTests/Extensions.cs Outdated Show resolved Hide resolved
TagCloudTests/FileTextHandlerTest.cs Outdated Show resolved Hide resolved
TagCloudTests/FileTextHandlerTest.cs Outdated Show resolved Hide resolved
TagCloudTests/GlobalUsings.cs Outdated Show resolved Hide resolved
TagCloudTests/TagCloudTests.csproj Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants