Skip to content

Ситдиков Сергей #259

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

Ситдиков Сергей #259
s1lenter wants to merge 2 commits intokontur-courses:masterfrom
s1lenter:master

Conversation

@s1lenter
Copy link
Copy Markdown

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Зачем эти файлы? Для демонстрации результата?

public class PrintingConfig<TOwner>
{
public class PrintingConfig<TOwner>
private readonly HashSet<Type> typesToExclude = new(ReferenceEqualityComparer.Instance);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Внутри эта коллеция не должна изменяться, лучше заменить на IReadOnlySet. Аналогично и для коллекций ниже. Каждый вызов метода конфига должен возвращать новый экземпляр, который не влияет на прошлые.


namespace ObjectPrinting;

public static class ObjectPrinterExtensions
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Эти два расширения можно объединить в одно

public class PrintingConfig<TOwner>
{
public class PrintingConfig<TOwner>
private readonly HashSet<Type> typesToExclude = new(ReferenceEqualityComparer.Instance);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

А для этого сета зачем Comparer передаём?

public class PrintingConfig<TOwner>
private readonly HashSet<Type> typesToExclude = new(ReferenceEqualityComparer.Instance);
private readonly HashSet<string> propertiesToExclude = [];
private readonly HashSet<object> processedObjects = new(ReferenceEqualityComparer.Instance);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Это же и так сет object, эта настройка в таком случае ничего нового не сделает

return $"{indentation}{propertyName} = {PrintToString(propertyValue, nestingLevel + 1)}";
}

private string SerializeEnumerable(IEnumerable enumerable, int nestingLevel)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

А если это какой-то вычисляемый ленивый IEnumerable? Что тут произойдёт тогда?

nestingLevel + 1));
var obj = objects[i];
serializeResult.Append($"{indentation}{PrintToString(obj, nestingLevel + 1)}");
if (i < objects.Count - 1)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Почему не использовать Join вместо этого?

[TestCase(-3)]
[TestCase(-4)]
[TestCase(-5)]
public void SetMaxNestingLevel_WithNegativeNestingLevel(int levelNesting)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Зачем столько тесткейсов?

[TestCase(0)]
[TestCase(1)]
[TestCase(2)]
public void SetMaxNestingLevel_WithDifferentNestingLevels(int levelNesting)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Не проверяется фактически что прекращается сериализация

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Названия тестов в файле не соответствует тому, чему вас учили на Testing. Лучше использовать <Название метода>_Should<Что должно выполняться>_When<При каких условиях>

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