Черных Илья #17
Черных Илья #17
Conversation
| @@ -0,0 +1,8 @@ | |||
| namespace TagsCloudVisualization.FileReaders; | |||
|
|
|||
| public interface IFileReader | |||
There was a problem hiding this comment.
Лучше сразу сделать IWordsReader, пускай возвращает коллекицю строк, по слову в строке
|
|
||
| namespace TagsCloudVisualization.Settings; | ||
|
|
||
| public class ImageSettings |
There was a problem hiding this comment.
Настройки цветов стоит либо расширить, либо сократить
TagsCloudVisualization/Tags/ITag.cs
Outdated
|
|
||
| namespace TagsCloudVisualization.Tags; | ||
|
|
||
| public interface ITag |
There was a problem hiding this comment.
Не стоит делать лишние интерфейсы. Для класса просто содержащего данные нет смысла создавать интерфейс
| this.fileReader = fileReader; | ||
| } | ||
|
|
||
| public Dictionary<string, int> HandleText() |
There was a problem hiding this comment.
Название GetWordsCount будет более точно отображать суть метода, даже если он будет содержать в себе дополнительную предобработку текста
| this.imageSettings = imageSettings; | ||
| } | ||
|
|
||
| public Bitmap Draw(IEnumerable<Tag> tags) |
There was a problem hiding this comment.
Не всегда стоит упарываться с абстракциями над типами, в большинстве случаев будет лучше указывать в аргументах конкретный тип с которым будешь работать, например список или массив. Если хочется обеспечить неизменность коллекции, то есть readonly типы колекций
| foreach (var paragraph in paragraphs) | ||
| { | ||
| var wordsInParagraph = Regex.Matches(paragraph, @"\b\w+\b") | ||
| .Select(word => word.Value); | ||
|
|
||
| words.AddRange(wordsInParagraph); | ||
| } |
There was a problem hiding this comment.
Сейчас есть 3 парсера, у которых повторяется один и тот же код. Стоит его переиспользовать
| this.readers = readers; | ||
| var reader = GetReader(settings.PathToText); | ||
|
|
||
| words = reader.Read(settings.PathToText); | ||
|
|
||
| var boringWordsReader = GetReader(settings.PathToBoringWords); | ||
|
|
||
| boringWords = boringWordsReader.Read(settings.PathToBoringWords).ToHashSet(); |
There was a problem hiding this comment.
Чтение файла является времязатратной операцией, стоит убрать её из конструктора. В целом стоит избегать продолжительных по времени операций в конструкторе, т.к. обычно ожидается, что он работает быстро
| foreach (var word in words) | ||
| { | ||
| var lowerWord = word.ToLower(); | ||
|
|
||
| if (wordsCount.TryGetValue(lowerWord, out var value)) | ||
| { | ||
| wordsCount[lowerWord] = ++value; | ||
| continue; | ||
| } | ||
| if (!boringWords.Contains(lowerWord)) | ||
| { | ||
| wordsCount[lowerWord] = 1; | ||
| } | ||
| } |
| var sortedWords = wordsCount | ||
| .OrderByDescending(p => p.Value) | ||
| .ToList(); |
There was a problem hiding this comment.
Стоит перенести в textHandler, и возвращать сразу список, если тебе не нужен словарь
TagsCloudVisualization/Program.cs
Outdated
| using TagsCloudVisualization.Di; | ||
| using TagsCloudVisualization.Visualization; | ||
|
|
||
| var commandLineOptions = Parser.Default.ParseArguments<CommandLineOptions>(args).Value; |
There was a problem hiding this comment.
Стоит корректно обработать аргумент --help и т.п.
There was a problem hiding this comment.
Так же стоит корректно завершать работу при вводе невалидных аргументов
| { | ||
| public bool CanRead(string pathToFile) | ||
| { | ||
| return pathToFile.Split('.')[^1] == "doc"; |
There was a problem hiding this comment.
При подобном сравнении текста по шаблонам, стоит преводить данные в единий регистр. Если расширение будет заглавными буквами, то чтение файла сломается
|
|
||
| foreach (var paragraph in paragraphs) | ||
| { | ||
| var wordsInParagraph = Regex.Matches(paragraph, @"\b\w+\b") |
There was a problem hiding this comment.
+w - включает в себя буквы, цифры и знак подчеркивания, из них словами можно считать только буквы. Предпологается, что во входных данных будут только слова, поэтому в данном случае всё ок. Если поправить, получится парсить текст в любом виде
@xsitin