Skip to content

Тимур Бабаев#263

Open
truefolder wants to merge 13 commits intokontur-courses:masterfrom
truefolder:homework
Open

Тимур Бабаев#263
truefolder wants to merge 13 commits intokontur-courses:masterfrom
truefolder:homework

Conversation

@truefolder
Copy link

No description provided.


public class ArchimedesSpiral(PointF center, float tightness, float distanceBetweenPoints) : ICoordinatesProvider
{
public IEnumerable<PointF> GetNextPoint()

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Название метода и выходные параметры не совпадают: по названию я ожидаю получить одну точку, а возвращается N.

Очень похоже, что ты пытался реализовать IEnumerator

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Изначально просто при проектировании думал, что буду возвращать каждый раз по точке, потом решил возвращать коллекцию точек, т.к. так удобнее как мне показалось, переименовал метод в GetPoints

Comment on lines 21 to 22
// ReSharper disable once IteratorNeverReturns
// TODO: это норм или нет?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ну, у тебя бесконечный цикл здесь, он и подсвечивает, что у тебя никогда полностью не вернется итератор

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Понял

Comment on lines 7 to 25
public static Rectangle ShiftToCenter(this Rectangle rectangle, Point center, List<Rectangle> otherRectangles)
{
var result = rectangle;
var canMoveX = true;
var canMoveY = true;

while (canMoveX || canMoveY)
{
canMoveX = TryShiftToCenterByX(result, center, otherRectangles, out var resultedMovementX);
canMoveY = TryShiftToCenterByY(result, center, otherRectangles, out var resultedMovementY);

if (canMoveX)
result.Offset(resultedMovementX);
if (canMoveY)
result.Offset(resultedMovementY);
}

return result;
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Как будто это не extensions должно быть, т.к ты много контекста передаешь сюда

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Поправил, теперь это утилита как и PolarCoordinatesUtils


namespace TagCloud.Visualizers;

[SuppressMessage("Interoperability", "CA1416:Проверка совместимости платформы")]

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Договорились переделать на кроссплатформенную либу

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Поправил

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants