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

Махлонов Дмитрий #210

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

Conversation

maakhhh
Copy link

@maakhhh maakhhh commented Dec 16, 2024

No description provided.

TagCloud/TagCloud/Program.cs Outdated Show resolved Hide resolved
TagCloud/TagCloud/Program.cs Outdated Show resolved Hide resolved
TagCloud/TagCloud/Program.cs Outdated Show resolved Hide resolved
TagCloud/TagCloud/Program.cs Outdated Show resolved Hide resolved
TagCloud/TagCloud/Program.cs Outdated Show resolved Hide resolved
TagCloud/TagCloud/TextFilters/ITextFilter.cs Outdated Show resolved Hide resolved

namespace TagCloud.TextReader;

public record class TextReaderSettings(string Path, Encoding Encoding);

Choose a reason for hiding this comment

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

Не очень хорошо передавать напрямую текст, так как таким ридером не получится воспользоваться дважды для разных файлов. Тогда лучше здесь передавать класс настроек

Copy link
Author

Choose a reason for hiding this comment

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

тут не совсем понял, там же не текст передается, а класс настроек в котором уже лежит путь до файла

TagCloud/TagCloud/TextReader/TxtTextReader.cs Outdated Show resolved Hide resolved
TagCloud/TagCloud/TextReader/TxtTextReader.cs Outdated Show resolved Hide resolved
TagCloud/TagCloudClient/Clients/IClient.cs Outdated Show resolved Hide resolved
TagCloud/TagCloud/BitmapGenerators/BitmapGenerator.cs Outdated Show resolved Hide resolved

public record BitmapGeneratorSettings
{
public Size ImageSize { get; private set; } = new Size(1080, 1920);

Choose a reason for hiding this comment

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

Из-за private set настройки стали по сути бесполезны, ведь ими нельзя управлять. Почему решил сделать именно так?

Copy link
Author

Choose a reason for hiding this comment

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

задумывал настройки неизменяемым объектом, если что-то нужно поменять - создаем новый объект настроек

TagCloud/TagCloud/BitmapGenerators/CloudWord.cs Outdated Show resolved Hide resolved
TagCloud/TagCloud/BitmapGenerators/IBitmapGenerator.cs Outdated Show resolved Hide resolved
Comment on lines +19 to +23
[TestCase(new string[1] { "hello" }, new string[1] { "hello" }, TestName = "Not skip simple words")]
[TestCase(new string[1] { "a" }, new string[0], TestName = "Skip articles")]
[TestCase(new string[3] { "at", "in", "on" }, new string[0], TestName = "Skip prepositions")]
[TestCase(new string[3] { "he", "she", "it" }, new string[0], TestName = "Skip pronouns")]
[TestCase(new string[2] { "at", "since" }, new string[0], TestName = "Skip conjunctions")]

Choose a reason for hiding this comment

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

Выглядит как самостоятельные тесты, которые в качестве проверки используют один и тот же метод. Соглашусь, что между кейсами и отдельными методами тонкая грань и каждый видит по своему. Скорее комментарий на подумать

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.

3 participants