Conversation
|
|
||
| namespace TagCloud.API | ||
| { | ||
| public class ConsoleAPI |
There was a problem hiding this comment.
Замечание про семантику. API - это интерфейс для программ. А это у тебя UI
| @@ -0,0 +1,11 @@ | |||
| namespace TagCloud.ReadWriter | |||
| { | |||
| public interface IReadWriter | |||
There was a problem hiding this comment.
Вот эту штуку я не понял. У тебя смешалась функциональность работы с файлами и с консолью, что, на мой взгляд, плохо. И то и другое - это чтение (интерфейс назван корректно). Но взаимодействие с консолью это UI, а с файлами - это функциональная часть библиотеки. Получается, что если захочется сделать GUI, то придется либо тащить туда что-то про консоль, либо переписывать код чтения файлов.
|
|
||
| public IEnumerable<Word> GetProcessedData(IEnumerable<string> wordsData, IEnumerable<string> boringWords) | ||
| { | ||
| var cache = new Dictionary<string, int>(); |
There was a problem hiding this comment.
Тут не очень clean code. Cache это очень абстрактно, лучше придумывать названия отражающие смысл в терминах твоей предметной области.
| cache[word]++; | ||
| } | ||
|
|
||
| var sortedWords = cache.OrderBy(wordCountPair => cache[wordCountPair.Key]); |
There was a problem hiding this comment.
wordCountPair => cache[wordCountPair.Key] проще wordCountPair => wordCountPair.Value
| var fontIncreaseByWordLevel = appConfig.FontConfig.FontIncreaseByWordLevel; | ||
| var defaultSize = appConfig.FontConfig.FontSize; | ||
|
|
||
| return defaultSize + sizeLevel * fontIncreaseByWordLevel; |
There was a problem hiding this comment.
На практике такой алгоритм не особо применим, в реальном тексте (особенно большом) разброс по встречаемости будет очень большой - второе слагаемое будет сильно отличаться.
Еще не очевидно, что относительные размеры слов разной встречаемости зависят от defaultSize. Если defaultSize маленький, то разброс будет большой, если defaultSize большой, то разброс наоборот маленький.
Я бы пробовал приращивать defaultSize по логарифмической функции.
| { | ||
| public interface ICloudLayouter | ||
| { | ||
| RectangleF GetPossibleNextRectangle(IEnumerable<WordTag> cloudRectangles, SizeF rectangleSize); |
There was a problem hiding this comment.
Смешение ответственности произошло. Лэйоутеру не нужны слова, он отдает прямоугольник, и работать должен с прямоугольниками
| return FindPossibleNextRectangle(cloudRectangles, rectangleSize); | ||
| } | ||
|
|
||
| private RectangleF FindPossibleNextRectangle(IEnumerable<WordTag> cloudRectangles, SizeF rectangleSize) |
There was a problem hiding this comment.
Каждый раз пытаешь класть прямоугольник в центр. - Действие после размещения первого прямоугольника бессмысленное. На большом количестве прямоугольников работать будет плохо.
| { | ||
| public interface ICloudDrawer | ||
| { | ||
| void DrawWordsAndSave(IEnumerable<WordTag> words); |
There was a problem hiding this comment.
Рисователь не должен заниматься сохранением, вместо этого от него хорошо бы получать массив байтов (картинку). Которую можно сохранять, отправлять по сети, кормить в другие алгоритмы и т. п.
| { | ||
| var wordsData = new List<string>() | ||
| { | ||
| "������", |
|
|
||
| namespace TagCloud.Config | ||
| { | ||
| public class AppConfig |
There was a problem hiding this comment.
Еще про эту штуку вспомнил. Такой класс с настройками плохо. Лучше было сделать отдельные классы конфигураций для каждого сервиса. + Настройки у тебя плохо инкапсулированы. Setter-ы надо делать приватными (или заменять на init)
2 participants
homework