Skip to content

Шубин Игорь#281

Open
Nevisinn wants to merge 2 commits intokontur-courses:masterfrom
Nevisinn:master
Open

Шубин Игорь#281
Nevisinn wants to merge 2 commits intokontur-courses:masterfrom
Nevisinn:master

Conversation

@Nevisinn
Copy link

Copy link

@Yrwlcm Yrwlcm left a comment

Choose a reason for hiding this comment

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

Сделал хорошо, мне понравилось, что все декомпозировал на классы со своими задачами - хорошая практика :)

И клево, что возможно другой наставник подсказал разбивать тесты и логику на разные проекты, тоже считай стандарт в разработке, я в прошлых домашках за это решил не душить

Какие-то еще свои мысли написал


private Rectangle PlaceNext(Size size)
{
while (true)
Copy link

Choose a reason for hiding this comment

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

Давай тут придумаем какое-то условие остановки, а то че-то while true страшновато выглядит. Хотя в целом и так норм в рамках учебного задания


var unitDirection = Vector2.Normalize(direction);
var offset = unitDirection * stepSize;
var testRect = moved with { X = (int)(moved.X + offset.X), Y = (int)(moved.Y + offset.Y) };
Copy link

Choose a reason for hiding this comment

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

Будто бы тут можем сожрать много памяти на том, что пересоздаем обьекты через with в цикле while true. Рекомендовал бы переделать через rect.Offset() для оптимизации, но тоже на твое усмотрение. Возможно читабельность упадет и заодно надо будет придумывать какие-нибудь TryOffset-ы

Comment on lines +16 to +17
var bitmap = new Bitmap(options.ImageSize.Width, options.ImageSize.Height);
var graphics = Graphics.FromImage(bitmap);
Copy link

Choose a reason for hiding this comment

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

Тут забыл задиспоузить битмапу и графику

TestContext.AddTestAttachment(filePath);
}


Copy link

Choose a reason for hiding this comment

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

nit: Лишний отступ)

{
var rectangles = new List<Rectangle>();

for (int i = 1; i < 100; i++)
Copy link

Choose a reason for hiding this comment

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

nit: var вместо int

}

[Test]
public void TearDown_ShouldCreateImage_WhenTestFail()
Copy link

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