Conversation
cs/TagsCloudVisualization/README.md
Outdated
| @@ -0,0 +1,3 @@ | |||
|  | |||
There was a problem hiding this comment.
А чего у тебя csproj нет у проекта? Или в гит просто не залился? Тяжело из-за этого смотреть код
|
|
||
| namespace TagsCloudVisualization; | ||
|
|
||
| public class WorkWithWorkWithFile : IWorkWithFile |
There was a problem hiding this comment.
Странное название класса. Зачем он вообще нужен в проекте?
There was a problem hiding this comment.
Убрал, согласен ток усложняет
| var stepX = currentRect.X + currentRect.Width / 2 < Center.X ? 1 : -1; | ||
| var stepY = currentRect.Y + currentRect.Height / 2 < Center.Y ? 1 : -1; | ||
|
|
||
| while ((currentRect.X + currentRect.Width / 2).CompareTo(Center.X) != stepX) |
There was a problem hiding this comment.
Какое-то немного странное условие. Что если мы шаг поменяем с 1 на 2? У нас вечный цикл будет? Можно как-то его поменять, чтоб таких приколов не было?
There was a problem hiding this comment.
Добавил условия на пересечение
| private readonly TextProcessor TextProcessor; | ||
| private readonly List<WordData> WordDataList; | ||
|
|
||
| public CloudRunner(Size imageSize, string outputPath, string wordsFilePath) |
There was a problem hiding this comment.
Тут не хотим установить максимум/минимум изображения, которое твоя программа сможет валидно обрабатывать?
Можно аналогичные проверки сделать на число слов/прямоугольников
| using var graphics = Graphics.FromImage(bitmap); | ||
| graphics.Clear(Color.White); | ||
|
|
||
| for (int i = 0; i < Rectangles.Count; i++) |
There was a problem hiding this comment.
Нигде не увидел, может быть такое что текст не поместится в квадратик?
| var fontSize = random.Next(MinSize, MaxSize); | ||
| using var wordFont = new Font(DefaultFont, fontSize); | ||
| var size = MeasureWordSize(word, wordFont); | ||
| var storedFont = (Font)wordFont.Clone(); |
There was a problem hiding this comment.
Убрал, он бы нужен потому что я прописал что
wordFont using сейчас он не using и я убрал
var storedFont = (Font)wordFont.Clone();
| throw new InvalidOperationException("Не удалось найти место для прямоугольника."); | ||
| } | ||
|
|
||
| private bool IsIntersection(Rectangle newRectangle) |
There was a problem hiding this comment.
Можно ли как-то быстрее чем за O(n) проверить что все прямоугольники не пересекаются?
There was a problem hiding this comment.
Вроде получилось сделать быстрее, сейчас проверяем только ячейки, свободны они или нет и если нет, то уже ток эти прямоугольники проверяем на пересечение
| private readonly Point Center; | ||
| private readonly List<Rectangle> PlacedRectangles = new(); | ||
| private readonly SpiralPointGenerator SpiralGenerator; | ||
| private const int Padding = 2; |
There was a problem hiding this comment.
Думаю что такие константы, и как например 0.1 ниже в коде можно явно у пользователя спрашивать.
Или например завести единый файл с конфигурацией твоего алгоритма, и брать их оттуда.
Чтобы можно было поигратся с параметрами если надо будет
There was a problem hiding this comment.
Добавил AppSettings где есть дефолтные значения и можно их менять
cs/TagsCloudVisualization/README.md
Outdated
| @@ -0,0 +1,3 @@ | |||
|  | |||
|  | |||
| gridSize = appSettings.MaxFontSize; | ||
| } | ||
|
|
||
| public Rectangle GetNextRectangle(Size rectangleSize) |
There was a problem hiding this comment.
Мелочь конечно, но переименуй в PutNextRectangle (по спецификации задачки)
| private readonly Dictionary<Point, List<Rectangle>> grid = new Dictionary<Point, List<Rectangle>>(); | ||
| private readonly int gridSize; | ||
|
|
||
| public CircularCloudLayouter(Point Center, AppSettings appSettings) |
|
|
||
| namespace TagsCloudVisualization; | ||
|
|
||
| public class SpiralPointGenerator |
There was a problem hiding this comment.
Что если я захочу переделать алгоритм генерации точек на какой-то другой. Легко ли его будет прокинуть в CircularCloudLayouter?
Может этот класс должен передаваться в него в конструкторе или еще как-то, и реализовывать общий интерфейс?
| { | ||
| var rect = rectangles[i]; | ||
| var data = wordDataList[i]; | ||
| using var brush = new SolidBrush(appSettings.WordColor); |
| if (PlacedRectangles.Count == 0) | ||
| { | ||
| var topLeft = CalculateTopLeft(Center, rectangleSize); | ||
| var CenterRect = new Rectangle(topLeft.X, topLeft.Y, rectangleSize.Width, rectangleSize.Height); |
| public CircularCloudLayouter(Point Center, AppSettings appSettings) | ||
| { | ||
| this.Center = Center; | ||
| this.appSettings = appSettings; |
| namespace TagsCloudVisualizationTests; | ||
|
|
||
| [TestFixture] | ||
| public class CircularCloudLayouterTests |
There was a problem hiding this comment.
Не хватает тестов:
Тест на максимальное количество прямоугольников
Тесты на различные размеры и соотношения сторон
Тесты на edge cases (размер 0, очень большие размеры)
Тесты на то, что действительно получается спираль
Тесты на плотность, что прямоугольники действительно близко стоят а не на рандом разбросаны
2 participants
No description provided.