Conversation
- написал рабочий PrintToString(c оговорками)
- написал рабочий вариант(не сериализует коллекции)
- реализовал принтинг коллекций(IEnumerable, IDictionary)
| { | ||
| private class Address | ||
| { | ||
| public string City { get; set; } = ""; |
There was a problem hiding this comment.
Зачем выносить класс для одного поля?
| namespace ObjectPrinterTests; | ||
|
|
||
| [TestFixture] | ||
| public class CollectionPrintingSnapshotTests |
There was a problem hiding this comment.
Стоит добавить тесты для граничных случаев (null-элементов, null-коллекций и т.д.)
ObjectPrinterTests/Helper.cs
Outdated
|
|
||
| public static class Helper | ||
| { | ||
| public static string Read(string fileName) |
There was a problem hiding this comment.
Какое преимущество перед готовыми инструментами? (Verify, ApprovalTests и т.д.). Если можно заиспользовать готовые решения, то лучше так и сделать
There was a problem hiding this comment.
Какое преимущество перед готовыми инструментами? (Verify, ApprovalTests и т.д.). Если можно заиспользовать готовые решения, то лучше так и сделать
Писал до второй лекции о тестах, поэтому не знал о них. Переделаю :)
| using FluentAssertions; | ||
| using NUnit.Framework; | ||
|
|
||
| namespace ObjectPrinting.Tests |
| [TestFixture] | ||
| public class ObjectPrinterTests | ||
| { | ||
| private class A |
There was a problem hiding this comment.
По смыслу тестовые данные ObjectPrinterTests, ObjectPrinterAcceptanceTests и CollectionPrintingSnapshotTests похожи. Можно ли использовать общие тестовые данные?
There was a problem hiding this comment.
По смыслу тестовые данные ObjectPrinterTests, ObjectPrinterAcceptanceTests и CollectionPrintingSnapshotTests похожи. Можно ли использовать общие тестовые данные?
По сути да, можно
| .Excluding(p => p.Id) | ||
| .Excluding<Guid>(); | ||
|
|
||
| //7. Синтаксический сахар в виде метода расширения, сериализующего по-умолчанию |
There was a problem hiding this comment.
Не понятно зачем эти комментарии + в этом тесте ничего не проверяется, текст просто выводится в консоль. Что должен проверять этот тест?
There was a problem hiding this comment.
Это тест из classwork, забыл удалить)
| } | ||
|
|
||
| [Test] | ||
| public void Demo() |
| @@ -1,12 +0,0 @@ | |||
| using System; | |||
|
|
|||
| namespace ObjectPrinting.Tests | |||
There was a problem hiding this comment.
Почему решил удалить эти тестовые данные и написать приватные классы?
There was a problem hiding this comment.
Чтобы можно было сделать Person с разными полями для разных типов тестов
Почему решил удалить эти тестовые данные и написать приватные классы?
| public static string PrintToString<TOwner>(object obj) | ||
| { | ||
| var printer = new PrintingConfig<TOwner>(); | ||
| return printer.PrintToString((TOwner)obj); |
There was a problem hiding this comment.
Почему в тестах используется не этот метод, а PrintingConfig.PrintToString? Для чего нужен этот метод?
There was a problem hiding this comment.
Почему в тестах используется не этот метод, а PrintingConfig.PrintToString? Для чего нужен этот метод?
Это синтаксический сахар в виде метода расширения, сериализующего по-умолчанию, когда в классе работали ребята просили реализовать
There was a problem hiding this comment.
Сейчас он не используется поэтому его можно убрать
| @@ -1,6 +1,9 @@ | |||
| <wpf:ResourceDictionary xml:space="preserve" xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml" xmlns:s="clr-namespace:System;assembly=mscorlib" xmlns:ss="urn:shemas-jetbrains-com:settings-storage-xaml" xmlns:wpf="http://schemas.microsoft.com/winfx/2006/xaml/presentation"> | |||
There was a problem hiding this comment.
давай исключим этот файл из PR через gitignore, т.к. не несет полезной информации
| Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "FluentMapping", "Samples\FluentMapper\FluentMapping.csproj", "{FEEA5AFE-459A-4D13-81D0-252E1A2E6F4E}" | ||
| EndProject | ||
| Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "FluentMapping.Tests", "Samples\FluentMapper.Tests\FluentMapping.Tests.csproj", "{8A7BB3EA-3E6A-4D04-A801-D5CD1620DA0D}" | ||
| EndProject |
There was a problem hiding this comment.
давай исключим этот файл из PR через gitignore, т.к. генерируется автоматически и не несет полезной информации
ObjectPrinting/ObjectExtensions.cs
Outdated
| var name = prop.Name; | ||
| var propType = prop.PropertyType; | ||
|
|
||
| if (config.ExcludedProperties.Contains(name) || |
There was a problem hiding this comment.
Сильная связность, extension метод знает все детали реализации конфига. Такого быть не должно, т.к. малейшие изменения в конфиге повлекут за собой изменения в extension методе
- вынес всю лишнюю логику из PrintingConfig.cs в PrintingConfigExtensions.cs
- переписал реализацию парсинга коллекций(теперь выводится генерик-тип и нет лишних символов)
|
|
||
| namespace ObjectPrinting.Extensions; | ||
|
|
||
| public static class CollectionPrintingExtensions |
There was a problem hiding this comment.
Теперь можно избавиться от этого класса и перенести методы в классы-стратегии
| dev | ||
| qa | ||
| ] | ||
| Addresses = Dictionary<String, Address> { |
There was a problem hiding this comment.
Хорошо бы переделать на языконезависимый JSON-подобный формат. Типы данных не должны быть явно прописаны, т.к. названия типов это детали C#. List<String> [ dev qa ] -> ["dev", "qa"]
No description provided.