HW #1
HW #1
Conversation
up .gitignore
add spiral vizualizer
finish task
reload examples
up readme
| [TestFixture] | ||
| internal class CircularCloudLayouter_Should | ||
| { | ||
| private static int GetCurrentTimeStamp() => (int) DateTime.UtcNow.Subtract(new DateTime(1970, 1, 1)).TotalSeconds; |
There was a problem hiding this comment.
TotalSeconds возвращает double
| [SetUp] | ||
| public void SetUp() | ||
| { | ||
| Cloud = new CircularCloudLayouter(new Point(300, 300)); |
There was a problem hiding this comment.
Почему Point а не Size?
Блин, понятно
There was a problem hiding this comment.
в конструкторе задается точка центра(условие задачи)
| namespace TagsCloudVisualization | ||
| { | ||
| [TestFixture] | ||
| internal class CircularCloudLayouter_Should |
| int seed) | ||
| { | ||
| GenerateRectangles(maxWidth, maxHeigth, numberOfRectangles, seed); | ||
| var result = new List<Size>(); |
| { | ||
| result = result | ||
| .Concat(GenerateRectangles(maxWidth, maxHeigth, numberOfRectangles, seed).Where(size => size.Width > size.Height)) | ||
| .ToList(); |
There was a problem hiding this comment.
тогда вот этот вызов .ToList() можно будет убрать`
There was a problem hiding this comment.
решарпер ругается на possible multiple enumeration
There was a problem hiding this comment.
@Griboedoff ну можно сделать лист из IEnumerable :D и возвращать этот лист .SelectMany...
There was a problem hiding this comment.
но нужно понимать, что ToList() каждый раз -- тоже не оч хорошо
| .Concat(GenerateRectangles(maxWidth, maxHeigth, numberOfRectangles, seed).Where(size => size.Width > size.Height)) | ||
| .ToList(); | ||
| } | ||
| return result.Take(numberOfRectangles); |
There was a problem hiding this comment.
Тем более, возвращаешь ты всё-равно Enumerable
| var rectangleSize2 = new Size(10, 10); | ||
| Cloud.PutNextRectangle(rectangleSize); | ||
| Cloud.PutNextRectangle(rectangleSize2); | ||
| Cloud.PlacedRectangles.Should().HaveCount(2); |
There was a problem hiding this comment.
Тут можно сделать свойство у облака -- сколько уже прямоугольников оно содержит
| public void NotPlace_IfRectIsOutsideCloudBorder() | ||
| { | ||
| var size = new Size(1000, 10); | ||
| Cloud.PutNextRectangle(size); |
There was a problem hiding this comment.
Тут и в других тестах лучше отделять стадии AAA пустыми строками. Так легче читать. Даже такие простые тесты :)
| // var rects = GenerateWordLikeRectangles(100, 30, 500, 10); | ||
| foreach (var square in rects) | ||
| cloud.PutNextRectangle(square); | ||
| var filename = Path.Combine( |
There was a problem hiding this comment.
Можно вынести это в отдельную функцию, и возможно даже получится сделать её однообразной для этого теста и для TearDown, в котором тоже вычисляется filename
| { | ||
| public static bool IsIntersectedWith(this Rectangle r1, Rectangle r2) | ||
| { | ||
| return r1.Bottom >= r2.Top && r1.Right >= r2.Left && r2.Bottom >= r1.Top && r2.Right >= r1.Left; |
| throw new ArgumentException($"Size must be positive {rectangleSize}"); | ||
|
|
||
| var nextSpiralPoint = spiral.GetNextSpiralPoint(); | ||
| var location = new Point(nextSpiralPoint.X - rectangleSize.Width / 2, nextSpiralPoint.Y - rectangleSize.Height / 2); |
There was a problem hiding this comment.
Лучше подобные штуки выносить в метод с говорящим названием. Ну и переменной тоже, наверное, можно подобрать более подходящее. А то не очень понятно, чей это location
There was a problem hiding this comment.
Имеется в виду, что не круто, когда для понимания чего-то простого нужно сначала посмотреть, где и для чего оно используется.
| var location = new Point(nextSpiralPoint.X - rectangleSize.Width / 2, nextSpiralPoint.Y - rectangleSize.Height / 2); | ||
|
|
||
| var nextRectangle = new Rectangle( | ||
| location, |
| nextSpiralPoint = spiral.GetNextSpiralPoint(); | ||
| nextRectangle = | ||
| new Rectangle( | ||
| new Point(nextSpiralPoint.X - rectangleSize.Width / 2, nextSpiralPoint.Y - rectangleSize.Height / 2), |
There was a problem hiding this comment.
кажется, где-то я уже видел такую же строчку кода
| PlacedRectangles = new List<Rectangle>(); | ||
| } | ||
|
|
||
| public Rectangle PutNextRectangle(Size rectangleSize) |
There was a problem hiding this comment.
кажется, что этот метод всё-таки можно ещё немного декомпозировать. Например, можно вытащить поиск подходящей позиции для прямоугольника.
| var verticalyMoved = true; | ||
| while (horizontalyMoved || verticalyMoved) | ||
| { | ||
| var vectToCenter = center.Sub(new Point(rect.Location.X + rect.Width / 2, rect.Location.Y + rect.Height / 2)); |
There was a problem hiding this comment.
Не экономь на спичках символах. Пиши полные имена переменных
| currentRadius = 0; | ||
| } | ||
|
|
||
| public Point GetNextSpiralPoint() |
TagsCloudVisualization/Spiral.cs
Outdated
| } | ||
| } | ||
|
|
||
| public class SpiralVizualizer : Form |
There was a problem hiding this comment.
Блин, давай всё-таки каждый класс в отдельном файле. А ещё лучше -- группировать файлы в директории.
TagsCloudVisualization/Spiral.cs
Outdated
| } | ||
|
|
||
| [TestFixture] | ||
| public class Spiral_Should |
There was a problem hiding this comment.
ну вот что-то на Spiral-то мало тестов :(
|
|
||
| public static bool IsInside(this Rectangle r1, Rectangle r2) | ||
| { | ||
| return r1.IsIntersectedWith(r2) && |
|
|
||
| namespace TagsCloudVisualization | ||
| { | ||
| public static class PointExtensions |
There was a problem hiding this comment.
эти Extension методы конечно простые, но всё равно лучше было бы их протестировать
| public List<Rectangle> PlacedRectangles { get; } | ||
|
|
||
| private bool IsInValidPosition(Rectangle checkingRect) | ||
| => !(PlacedRectangles.Any(rect => rect.IsIntersectedWith(checkingRect) || checkingRect.IsInside(rect)) |
There was a problem hiding this comment.
о, вот кстати, может быть такое, что все уже расставленные прямоугольники будут внутри checkingRect?
Changed some methods to build-in
Add throwing exception if cloud is too small (add test too)
No description provided.