Skip to content

Зубков Артём #252

Open
artem7841 wants to merge 3 commits intokontur-courses:masterfrom
artem7841:master
Open

Зубков Артём #252
artem7841 wants to merge 3 commits intokontur-courses:masterfrom
artem7841:master

Conversation

@artem7841
Copy link
Copy Markdown

Comment on lines +13 to +23
string result;
if (str.Length > maxLength)
{
result = str.Substring(0, maxLength);
}
else
{
result = str;
}

return result;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

nit: можно заменить result на return и/или схлопнуть в тернарник

[SetUp]
public void Setup()
{
testPerson = new Person
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

nit: я бы вынес это в статическую фабрику, так бы получилась более явная гарантия, что между тестами не остается общих данных

</PropertyGroup>

<ItemGroup>
<PackageReference Include="FluentAssertions" Version="8.8.0" />
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Точно ли это здесь нужно? Эта зависимость обычно используется только для тестов

</PropertyGroup>

<ItemGroup>
<PackageReference Include="FluentAssertions" Version="8.8.0" />
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

То же самое


namespace ObjectPrinting
{
public class ObjectPrinter
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Выглядит как статический класс


namespace ObjectPrinting
{
public class PrintingConfig<TOwner>
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

nit: Обычно делаю все классы по умолчанию sealed. Это такая холиварная нанооптимизация + заставляет лишний раз подумать, прежде чем расширять какой-либо класс

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