-
Notifications
You must be signed in to change notification settings - Fork 213
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Алешев Руслан #202
base: master
Are you sure you want to change the base?
Алешев Руслан #202
Conversation
using System.Text; | ||
|
||
namespace ObjectPrinting | ||
{ | ||
public class PrintingConfig<TOwner> | ||
{ | ||
private readonly HashSet<PropertyInfo> excludedProperties = new HashSet<PropertyInfo>(); | ||
private readonly HashSet<Type> excludedTypes = new HashSet<Type>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Думаю, можно воспользоваться короткой формой = new();
. Если нет, то и нет, но можно поднять .net
и версию C#
There was a problem hiding this comment.
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="6.12.0" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Давай для тестов заводить отдельный проект, чтоб не мешать тесты и какую-либо логику. Плюсом в основном коде не будут доступны тестовые классы/методы (если правильно настроить область видимости), размер сборок станет меньше, проще будет запускать тесты отдельно и т.д.
Для Rider
'а:
Добавляешь папку Tests
: [имя твоего решения] -> клик ПКМ -> New Solution Project
.
Делаешь вот так, указываешь нужное имя в Project name
.
{ | ||
excludedTypes.Add(typeof(TProp)); | ||
return this; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Перед сдачей решения запуская, пожалуйста, форматирование кода. Для Rider
'a это сочетание ctrl + alt + L
/ shift + ctrl + alt + L
. Либо можно настроить автоформатирование на сохранение кода ctrl s
. Для Visual Studio
не подскажу, но это гуглится. В коде оч много мест, где следовало бы применить форматирование.
public PrintingConfig<TOwner> Exclude<TProp>() | ||
{ | ||
excludedTypes.Add(typeof(TProp)); | ||
return this; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Оставляй перед return
пустую строку. И сразу проверь везде, пж)
Это делается для того, чтоб было сразу и четко ясно, что и где возвращается из метода.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✔️
var values = dictionaryValue.Values.OfType<object>().Select(item => item.ToString()).ToArray(); | ||
var pair = keys.Zip(values).Select(pair => string.Format("({0}, {1})", pair.First, pair.Second)); | ||
|
||
return $"{propertyInfo.Name} = {{ " + string.Join(", ", pair) + " }"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Тут можно использовать интерполяцию целиком, без конкатенации.
var items = listValue.OfType<object>().Select(item => item.ToString()).ToArray(); | ||
return $"{propertyInfo.Name} = {{ " + string.Join(", ", items) + " }"; | ||
} | ||
|
||
private string PrintToString(object obj, int nestingLevel) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Кажется, что реализация сериализации должна лежать не в Printing
Config
using FluentAssertions; | ||
using System; | ||
using System.Globalization; | ||
using System.Collections.Generic; | ||
|
||
namespace ObjectPrinting.Tests | ||
{ | ||
[TestFixture] | ||
public class ObjectPrinterAcceptanceTests |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Нигде нет структуры ААА
[Test] | ||
public void PrintToString_SkipExcludedPropertys() | ||
{ | ||
var printer = ObjectPrinter.For<Person>().Exclude(x=>x.Age); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Часто тестируемый модуль называют sut
- system under test
. Так проще ориентироваться в тесте.
Вообще, ещё есть cut
- class under system
. Пишут так крайне редко (по крайней мере, исходя из моего опыта), но для полноты картинки пусть будет.
public void PrintToString_ParsinCyclingLinks() | ||
{ | ||
var parent = new Person(); | ||
parent.Id = new Guid("00000000-0000-0000-0000-012147483648"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Почему бы не использовать инициализатор?
var parent = new Person {Id = new Guid("00000000-0000-0000-0000-012147483648")};
?
{ | ||
var printer = ObjectPrinter.For<Person>().Serialize<double>().Using<Double>(CultureInfo.InvariantCulture); | ||
var serialized = printer.PrintToString(person); | ||
serialized.Should().Be("Person\r\n\tId = 00000000-0000-0000-0000-000000000000\r\n\tName = Alex\r\n\tHeight = 12.34\r\n\tAge = 19\r\n\tparent = null\r\n"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
А можно ли применить культуру к всей сериализации? Т.е. сразу ко всем типам данных?
@pakapik