Skip to content

Тимур Бабаев#216

Open
truefolder wants to merge 10 commits intokontur-courses:masterfrom
truefolder:homework
Open

Тимур Бабаев#216
truefolder wants to merge 10 commits intokontur-courses:masterfrom
truefolder:homework

Conversation

@truefolder
Copy link
Copy Markdown

No description provided.

Copy link
Copy Markdown

@Pasha0666 Pasha0666 left a comment

Choose a reason for hiding this comment

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

И у тебя появилась новая инфра в виде Result, но тестов на неё не добавилось

Comment on lines +35 to +36
}
public static Result<None> Ok()
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

Choose a reason for hiding this comment

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

Поправил

Comment on lines +101 to +107
public static Result<TInput> OnFail<TInput>(
this Result<TInput> input,
Action<string> handleError)
{
if (!input.IsSuccess) handleError(input.Error);
return input;
}
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

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.

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

Comment on lines +19 to +20
return creator.Create(options);
}).Then(x => x);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Для чего строчка .Then(x => x);?

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.

Вообще конечно выглядит костыльно, но в блоке кода выше я возвращаю Result, и в конечном итоге получается Вложенный Result в Result, и с помощью Then я его "распаковываю". Переделал не оборачивая полностью выражение в Result.Of.

@truefolder
Copy link
Copy Markdown
Author

Еще добавил тесты на сам Result

Comment on lines +17 to +18
return creator.Create(options)
.Then(Result.Ok);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

А зачем этот .Then(Result.Ok) появился?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

И пробегись по остальным местам где была такая же конструкция .Then(x => x)

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