Skip to content

Кроткая Александра#241

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

Кроткая Александра#241
Krotkaya wants to merge 3 commits intokontur-courses:masterfrom
Krotkaya:master

Conversation

@Krotkaya
Copy link
Copy Markdown

{
if (obj == null) return "null" + Environment.NewLine;

if (visitedObjects.Contains(obj))
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
Author

@Krotkaya Krotkaya Nov 18, 2025

Choose a reason for hiding this comment

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

Не очень поняла, что ты имеешь в виду
В этой строке
if (obj == null) return "null" + Environment.NewLine;
я предубеждаю NullReferenceException так как ниже беру его тип obj.GetType(). А у нас поле в классе вполне может где-то содержать null

А благодаря этой
if (visitedObjects.Contains(obj))
я ловлю циклические ссылки, чтобы не словить переполнение стека

Поправь меня пожалуйста, если я неправа

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Тут больше вопрос в стилистике кода. Посмотри на сигнатуру метода Add у хешсета

private Person? testPerson;

[SetUp]
public void Setup()

This comment was marked as resolved.

namespace ObjectPrinting.Tests;

[TestFixture]
public class ObjectPrinterTests

This comment was marked as resolved.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

У меня есть один тест на вложенность PrintToString_WithPersonList_ShouldPrintList (последний), но я добавила еще

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Да, увидел. Имел ввиду тесты, где есть объекты, вложенные в объекты и т.д.


return PrintObject(obj, nestingLevel, type);
}
finally

This comment was marked as resolved.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

type.GetFields и type.GetProperties в PrintObject () могут нам кинуть ошибки с рефлексией, это системные ошибки и их лучше пробросить дальше или "развалиться" :)
Также могут прилететь ошибки из сериализатора SerializeValue(), на этом уровне мне кажется лучше всего предупредить пользователя о том, что какой-то объект не сериализовался => я ловлю ошибку в методе SerializeValue()

@Luvr681

This comment was marked as resolved.

@Krotkaya
Copy link
Copy Markdown
Author

Krotkaya commented Nov 18, 2025

Общий вопрос, не относящийся к коду напрямую Можешь подсказать, пожалуйста, почему выложила одним коммитом?

В проектах, которые я делаю одна, я обычно делаю локальные коммиты в случае, если:

  1. Сделала какую-то таску/часть проекта, которае выполняет свои задачи (это скорее для более крупных проектов, где много модулей), и ее можно зафиксировать, как законченное на данном этапе решение этого блока
  2. Есть какая-то работающая логика, я хочу попробовать внести изменения, в которых не уверена, и они могут затронуть работающую логику. Тут тоже я делаю коммит, чтобы откатиться, если моя идея не сработает :)

Поэтому в данном случае, так как я работаю одна, и в связи с тем, что задача не крупная + у нас были интерфейсы, задающие архитектуру, мне не понадобились коммиты
Подскажи пожалуйста, лучше было закоммитить отдельно реализацию PrintingConfig и других классов и отдельно тесты?

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