Skip to content

Русинов Матвей#245

Open
nbdevncrs wants to merge 29 commits intokontur-courses:masterfrom
nbdevncrs:master
Open

Русинов Матвей#245
nbdevncrs wants to merge 29 commits intokontur-courses:masterfrom
nbdevncrs:master

Conversation

@nbdevncrs
Copy link
Copy Markdown

…es that chosen and creates tree with those properties
…r string with all chosen serializations and returns it to ObjectPrinter.PrintToString()
…ogic, added it to PrintToString in PrintingConfig.cs, new logic completely implemented
return printingConfig;
}

public PrintingConfig<TOwner> Use(IFormatProvider formatProvider)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Можно тоже ограничить применение по типу. Зачем мне культура для символа например?


internal PropertyPrintingConfig(PrintingConfig<TOwner> parentConfig, Expression<Func<TOwner, TPropType>> selector)
{
printingConfig = parentConfig ?? throw new ArgumentNullException(nameof(parentConfig));
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Прям везде проверять на null можно, но не обязательно. Я бы склонялся к тому, что всё должны описывать сигнатуры. Если в сигнатуре поле !nulalble а кто-то прокидывает туда его - это на его совести(и ему же огребать в большинстве случаев)

public static class PropertyPrintingConfigExtensions
{
public static PrintingConfig<TOwner> Trim<TOwner>(
this PropertyPrintingConfig<TOwner, string> config,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Почему нельзя обрезать все строки, а только конкретные пропы?


namespace ObjectPrinting.PrintingHandlers.ApplyingSettings.Appliers;

internal class CultureApplier : ISettingsApplier
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Обычно в названии имплементации пишут полное название интерфейса CultureSettingsApplier. Да и так просто понятнее, что делает этот класс. А то он просто "применитель"

!context.Settings.StringTrimLengths.TryGetValue(context.Path, out var max))
return ApplierResult.NotApplied;

var trimmed = s.Length <= max ? s : s[..max];
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Любые работы со строками дорогие. Как насчёт использовать Span?

else if (printed.Contains(Environment.NewLine))
{
sb.Append(prefix).Append($"[{i}] = ").AppendLine();
var lines = printed.Split(["\r\n", "\n"], StringSplitOptions.None);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

\r\n/\n
Это же всё равно зависит от Environment.NewLine. Я бы предложил оставить только его

sb.Append(prefix).Append($"[{i}] = ").AppendLine();
var lines = printed.Split(["\r\n", "\n"], StringSplitOptions.None);
foreach (var line in lines)
sb.Append(prefix).Append('\t').AppendLine(line);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Почему бы не добавлять prefix в самом printed? Чтобы Enumerable им сразу говорил что их Indent больше, тк они часть коллекции.
А то получаются какие-то лишние и неочевидные пересборки строк

{
public bool CanHandle(ValueContext context)
{
switch (context.Value)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Почему не if (context.Value is null or string) return false? Такой switch тяжелее читать


var type = context.Type;
if (type == null) return true;
if (type.IsPrimitive) return false;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

логика проверки на Примитив используется уже в 2 местах. Можно вынести в helper/etc

{
sb.Append(prefix).Append(p.Name).Append(" = ").AppendLine();
var childIndent = new string('\t', context.Indent + 2);
var lines = printed.Split(["\r\n", "\n"], StringSplitOptions.None);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Аналогично с Enumerable

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