Alexander Andronov #203
Alexander Andronov #203
Conversation
|
|
||
| namespace ObjectPrinting | ||
| { | ||
| public class Options |
There was a problem hiding this comment.
- Рекомендую всегда и все свои классы делать
sealed(можно даже в шаблоне к классу это добавить). В некоторых ситуациях это микроскопически улучшает производительность, но что более важно - напоминает более тщательно продумывать иерархию наследования - Поля в подобных классах обычно делают пропами. Это поможет закрыть следующую рекомендацию - параметры желательно ограничить для редактирования. В идеале они должны быть
init-only. Это даст хоть какую-то гарантию где-то в неожиданном месте не произойдет мутация и не внесет суету в поведение. Можно заморочиться и ввести еще и интерфейс, с аналогичными полями но толькоgetterами, тогда у тебя будет абсолютная иммутабельность и полиморфность
| if (obj == null) | ||
| return "null"; | ||
|
|
||
| if (obj.GetType() == typeof(string) && configuration.Options.MaxStringLength != -1) |
|
|
||
| foreach (var propertyInfo in properties) | ||
| { | ||
| var serialized = ""; |
There was a problem hiding this comment.
Любой другой вариант чуть лучше. Иначе выглядит так, будто бы забыли что-то вписать в кавычки
var serialized = string.Empty;var serialized = default(string);string serialized;
|
|
||
| var identation = new string('\t', nestingLevel + 1); | ||
| var type = obj.GetType(); | ||
| var lines = new List<string>() { type.Name}; |
There was a problem hiding this comment.
Было бы более оптимально заменить на StringBuilder
| serialized = PrintToString(propertyInfo.GetValue(obj), | ||
| nestingLevel + 1); | ||
|
|
||
| var line = string.Format("{0}{1} = {2}", |
There was a problem hiding this comment.
Вероятнее всего тоже лучше склеивать через билдер, но если нет, то хотя бы так
var line = $"{identation}{propertyInfo.Name} = {serialized}";| { | ||
| if (obj is IDictionary) | ||
| { | ||
| var list = new List<string>(); |
| return string.Join("\n", lines); | ||
| } | ||
|
|
||
| private string PrintCollection(object obj) |
There was a problem hiding this comment.
Подразумевается, что сюда будут отправляться только нечто наследуемое от ICollection. Сейчас проблема может быть с непреднамеренной передачей сюда чего-нибудь левого. Поэтому два варианта на выбор.
- Заменить
objectнаICollectionи обязать данный метод сериализовать буквально любые коллекции - Переделать метод в формат типа
bool TryPrintCollection(object obj, out string result), который будет принимать любой объект, анализировать тип и при удачно стечении обстоятельств отдавать какой-то результат
| serializer = configuration.Serializers | ||
| .FirstOrDefault(x => x.predicate(info)) | ||
| .serializer; | ||
| return !(serializer is null); |
There was a problem hiding this comment.
nit: Не люблю отрицания через !, стараюсь всячески их избегать, т.к. часто неприглядно выглядят и могут создавать какую-то запутанную логику с отрицаниями. Поэтому я бы сделал return serializer is not null
| { | ||
| private Configuration configuration; | ||
|
|
||
| private readonly Type[] finalTypes; |
There was a problem hiding this comment.
Лучше заиспользоваться что-нибудь IReadonly*, иначе мало ли кто и почему захочет поменять какое-нибудь значение в массиве где-нибудь в середине кода. Судя по тому что коллекция используется для проверки наличия типов в списке, оптимальнее было бы взять IReadonlySet + HashSet
No description provided.