Skip to content

Тимур Бабаев#38

Open
truefolder wants to merge 21 commits intokontur-courses:masterfrom
truefolder:homework
Open

Тимур Бабаев#38
truefolder wants to merge 21 commits intokontur-courses:masterfrom
truefolder:homework

Conversation

@truefolder
Copy link
Copy Markdown

No description provided.

Copy link
Copy Markdown

@Pasha0666 Pasha0666 left a comment

Choose a reason for hiding this comment

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

как минимум не вижу теста на DI что он работает и собирает все что нужно
ну и в целом тестов маловато

Comment on lines +52 to +65
builder.RegisterType<WordSingleColorizerCreator>()
.Keyed<IWordColorizerCreator>("single")
.SingleInstance();
builder.RegisterType<WordPaletteColorizerCreator>()
.Keyed<IWordColorizerCreator>("palette")
.SingleInstance();
builder.RegisterType<WordGradientColorizerCreator>()
.Keyed<IWordColorizerCreator>("gradient")
.SingleInstance();
builder.RegisterType<HexColorParser>()
.As<IColorParser>()
.SingleInstance();
builder.RegisterInstance(new TagCloudVisualizer(Color.AntiqueWhite))
.As<ITagCloudVisualizer>();
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

А почему в DI константы проникли? которые должен по идее пользователь задавать

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Поправил

Comment on lines +3 to +5
using CommandLine;
using TagCloud.Client;
using TagCloud.Client.CLI;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

почисти юзинги, еще видел в файлах других

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Поправил

Comment on lines +13 to +21
public class TagCloudModule(string boringWordsPath) : Module
{
protected override void Load(ContainerBuilder builder)
{
builder.RegisterType<WordsProviderResolver>().As<IWordsProviderResolver>().SingleInstance();
builder.RegisterType<TxtWordsProvider>().As<IWordsProvider>().SingleInstance();
builder.RegisterType<DocWordsProvider>().As<IWordsProvider>().SingleInstance();
builder.RegisterType<DocxWordsProvider>().As<IWordsProvider>().SingleInstance();
builder.RegisterType<WordLowercaser>()
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Ну и более важный вопрос, а с такой регистрацией получиться сделать GUI?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Теперь не зависим от каких-либо параметров/констант при регистрации типов, вполне сойдет для вынесения в отдельный core-модуль регистрации в дальнейшем

Comment on lines +3 to +6
public interface IWordsProviderResolver
{
IEnumerable<string> ReadWords(string path);
} No newline at end of file
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Обычно Resolver отдает сущность, а не делает действие

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Поправил, теперь Resolver отдает IWordsProvider'а

Comment on lines +5 to +12
public class WordProcessor(IWordNormalizer normalizer, IWordFilter filter) : IWordProcessor
{
public IEnumerable<string> Process(IEnumerable<string> words) =>
words.SelectMany(text => Regex.Split(text, @"\P{L}+")) // защита от мусора в docx/doc файлах, сплитим только по небуквенным символам
.Select(normalizer.Normalize)
.Where(word => !string.IsNullOrWhiteSpace(word))
.Where(filter.IsValid);
} No newline at end of file
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Не вижу исключения скучных слов

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

На 11 строке использую filter, который IWordFilter, который регается в DI как BoringWordsFilter

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Апдейт, теперь регистрируем фабрику фильтров, но всё равно в конечном итоге в IWordFilter попадает BoringWordsFilter

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