Skip to content

Головнев Максим#50

Open
maximka200 wants to merge 8 commits intokontur-courses:masterfrom
maximka200:master
Open

Головнев Максим#50
maximka200 wants to merge 8 commits intokontur-courses:masterfrom
maximka200:master

Conversation

@maximka200
Copy link
Copy Markdown

No description provided.

@maximka200
Copy link
Copy Markdown
Author

@Dimques


actualTsar.Parent.Should().BeEquivalentTo(expectedTsar.Parent, options =>
options.Excluding(t => t.Id)
.Excluding(t => t.Parent));
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Одна из основных задач тестов - грохнуться, если разработчик случайно (или намеренно) что-то сломал, дорабатывая функциональность. Представь, что в методе TsarRegistry.GetCurrentTsar() в качестве деда Ивана Грозного кто-то добавил Петра Первого - твой тест будет зелёным, а вот исходный вариант успешно покраснеет. Как бы ты исправил это?

Copy link
Copy Markdown
Author

@maximka200 maximka200 Oct 29, 2025

Choose a reason for hiding this comment

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

Нашел поле Path у IMemberInfo, в котором через метод EndWith, получается исключить Id у всех царей в цепочке и сделать проверку через один BeEquivalentTo. Но есть минус: если вдруг при расширении появится другое Id (например, DynastyId), то исключится и оно

Одна из основных задач тестов - грохнуться, если разработчик случайно (или намеренно) что-то сломал, дорабатывая функциональность. Представь, что в методе TsarRegistry.GetCurrentTsar() в качестве деда Ивана Грозного кто-то добавил Петра Первого - твой тест будет зелёным, а вот исходный вариант успешно покраснеет. Как бы ты исправил это?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Слишком глубоко копаешь, я пытаюсь тебя на гораздо более простую мысль натолкнуть. Кмк, оставить нерекурсивную проверку ок, потому что сейчас проверяемый метод содержит только отца и сына, но тест не проверяет ничего сверх этого. Как минимум, хотелось бы, чтобы тест просто грохнулся, если вдруг появился дед (неважно, с прадедом или без). Это простое и дешевое решение, и я уверен, что в реальном проекте средний программист в вакууме написал бы именно так.

Но раз уж ты заговорил про рекурсивную проверку, то подумай еще, как исключить поле Id на произвольной глубине. Намекну - EndsWith плох ещё и тем, что ты туда строку передаёшь, т.е. хардкодишь имя поля.

Copy link
Copy Markdown

@Dimques Dimques Oct 29, 2025

Choose a reason for hiding this comment

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

EndsWith плох ещё и тем, что ты туда строку передаёшь, т.е. хардкодишь имя поля.

Здесь неудачно выразился. Здесь главное, что хардкодить имя поля, например, "Id" при передаче в тот же EndsWith неправильно, т.к. оно может измениться. В общем, намёк всё жирнее становится :)

Copy link
Copy Markdown
Author

@maximka200 maximka200 Oct 29, 2025

Choose a reason for hiding this comment

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

если брать имя поля через nameof, то это безопаснее, т.к. в случае если имя поля поменяется, то тест у нас просто не запустится

EndsWith плох ещё и тем, что ты туда строку передаёшь, т.е. хардкодишь имя поля.

Здесь неудачно выразился. Здесь главное, что хардкодить имя поля, например, "Id" при передаче в тот же EndsWith неправильно, т.к. оно может измениться. В общем, намёк всё жирнее становится :)

Comment thread Testing/Basic/Homework/2. NumberValidator/NumberValidatorTests.cs Outdated
Comment thread Testing/Basic/Homework/2. NumberValidator/NumberValidatorTests.cs Outdated
Comment thread Testing/Basic/Homework/2. NumberValidator/NumberValidatorTests.cs Outdated
Comment thread Testing/Basic/Homework/2. NumberValidator/NumberValidatorTests.cs Outdated
Comment thread Testing/Basic/Homework/2. NumberValidator/NumberValidatorTests.cs Outdated
Comment thread Testing/Basic/Homework/2. NumberValidator/NumberValidatorTests.cs Outdated
Comment thread Testing/Basic/Homework/2. NumberValidator/NumberValidatorTests.cs Outdated
Comment thread Testing/Basic/Homework/2. NumberValidator/NumberValidatorTests.cs Outdated
Comment thread Testing/Basic/Homework/2. NumberValidator/NumberValidatorTests.cs Outdated
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