-
Notifications
You must be signed in to change notification settings - Fork 32
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
Брайденбихер Виктор Николаевич #9
base: master
Are you sure you want to change the base?
Conversation
TagCloud/TagCloudCreator.cs
Outdated
{ | ||
public class TagCloudCreator | ||
{ | ||
public readonly ICloudLayouter Layouter; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
А зачем public
TagCloud/Visualisers/TagCloudWord.cs
Outdated
public class TagCloudWord(Rectangle box, string textWord, int fontSize) | ||
{ | ||
public readonly Rectangle Box = box; | ||
|
||
public readonly string TextWord = textWord; | ||
|
||
public readonly int FontSize = fontSize; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
public readonly не принято писать. Если данные торчат наружу, то их принято оборачивать в свойство public Rectangle Box { get; }
Зачем: считается, что так мы можем более четко обозначить уровень доступа к свойству. Например, можно вообще убрать get и оставить только set, то есть клиент может записать свойство, но не посмотреть значение
А тут, если ты убрал конструктор наверх, можно вообще заменить на record TagCloudWord(Rectangle Box, string TextWord, int FontSize)
ConsoleClient/Program.cs
Outdated
//WordReaders | ||
builder.RegisterType<WordReaderFactory>().As<IWordReaderFactory>().SingleInstance(); | ||
builder.RegisterType<MultiFormatWordReader>().As<IWordReader>(); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Вот эти все регистеры убрать бы в отдельный файл. И заодно сделать приватными и создать один публичный метод RegisterAll
ImageSaveSettings imageSaveSettings, | ||
ImageCreateSettings imageCreateSettings, | ||
IWordReaderFactory wordReaderFactory, | ||
WordReaderSettings wordReaderSettings, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Все settings можно было передать в метод, а не в зависимости
|
||
public class BitmapCreator : IBitmapCreator | ||
{ | ||
public static Bitmap GenerateImage(IEnumerable<TagCloudWord> cloudWords, ImageCreateSettings settings) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Я чет не понял зачем вообще тогда интерфейс если метод статичный
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Осознал, в чем неправильность статических методов при реализации интерфейсов в моем случае, исправил
public IEnumerable<string> Read() | ||
{ | ||
return wordReader.Read(); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Чет не понял как это должно работать. wordReader же не инициализируется
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Сам не знаю, исправил это
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if (words == null) throw new ArgumentNullException(nameof(words), "Input words cannot be null."); | ||
|
||
var filteredWords = words; | ||
foreach (var filter in _filterStrategies) filteredWords = filter(filteredWords); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Для этого есть Aggregate: filteredWords = _filterStrategies.Aggregate(words, (words, filter) => filter(words));
|
||
if (Enum.TryParse(partOfSpeechStr, out PartOfSpeech partOfSpeech)) return partOfSpeech; | ||
|
||
return default; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
дефолтное значение енума - первое значение в списке, то есть прилагательное
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Сделал UNKWON, в дальнейшем это значение отфильтровывается
@Inree, реализовал структуру решения, часть классов имеет конкретную протестированную функциональность.
DI контейнер планирую использовать Autofac.
В качестве клиента собираюсь создать оконное приложение используя WPF
Следующие пункты на перспективу собираюсь реализовать: