Skip to content
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

Яценко Ирина #198

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

lholypeachy
Copy link

No description provided.

@@ -1,40 +1,156 @@
using System;
Copy link

Choose a reason for hiding this comment

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

Все комменты обсуждаемы - можем обсуждать или в тредах, или в личке в телеге
За количество комментов не парься - это нормально, процесс обучения, повышения скилла, ревью именно для этого и нужно

На пофикшенные комментарии можно ставить реакции или писать пометочки, что и почему

{
Func<string, string> func = value => value.Length <= length
? value
: value.Substring(0, length);

This comment was marked as resolved.

if (finalTypes.Contains(obj.GetType()))
return obj + Environment.NewLine;

if (!complexObjectLinks.ContainsKey(obj))

This comment was marked as resolved.

private string PrintToString(object obj, int nestingLevel)
{
//TODO apply configurations
if (obj == null)
return "null" + Environment.NewLine;

This comment was marked as resolved.

complexObjectLinks[obj]++;

if (complexObjectLinks[obj] == maxRecursion)
return "Maximum recursion has been reached" + Environment.NewLine;

This comment was marked as resolved.

}

[Test]
public void TrimString_ShouldReturnSubstring()

This comment was marked as resolved.

public class ObjectPrinterAcceptanceTests
{
[Test]
public void WhenExcludeType_ShouldReturnWithoutThisType()

This comment was marked as resolved.

namespace ObjectPrintingTests
{
[TestFixture]
public class ObjectPrinterAcceptanceTests

This comment was marked as resolved.


if (membersSerializesInfos.TryGetValue(propertyInfo, out var serializeMember))
{
var t = serializeMember(propertyInfo.GetValue(obj));

This comment was marked as resolved.


if (complexObjectLinks[obj] == maxRecursion)
return "Maximum recursion has been reached" + Environment.NewLine;

var identation = new string('\t', nestingLevel + 1);
Copy link

@gisinka gisinka Dec 25, 2023

Choose a reason for hiding this comment

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

Го заюзаем ради интереса string.Intern(), это позволит избежать лишних аллокаций

Если хранить в ста или, что еще хуже, в миллионе строковых переменных одинаковое значение получится, что память для хранения значений строк будет выделяться снова и снова. Интернирование строки это способ обойти эту проблему

sb.Append(identation + propertyInfo.Name + " = " +
PrintToString(propertyInfo.GetValue(obj),
nestingLevel + 1));
DataMember data = new DataMember(propertyInfo);

This comment was marked as resolved.

}

private void HandleMember(StringBuilder sb, DataMember member, object obj, string indentation, int nestingLevel)

This comment was marked as resolved.

sb.Append($"{indentation}{propertyInfo.Name} = " +
PrintToString(propertyInfo.GetValue(obj),
nestingLevel + 1));
DataMember data = new DataMember(propertyInfo);
Copy link

Choose a reason for hiding this comment

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

у тебя просто propertyInfo заворачивается в DataMember - зачем? Почему бы не просто передавать MemberInfo в метод

А еще можно сделать финт ушами, чтобы по разному не обрабатывать поля и свойства:

const BindingFlags bindingFlags = BindingFlags.Public | BindingFlags.Instance;
var fields = type.GetFields(bindingFlags);
var properties = type.GetProperties(bindingFlags);

return fields.Concat<MemberInfo>(properties);

using System.Linq;
using System.Linq.Expressions;
using System.Reflection;
using System.Text;

namespace ObjectPrinting
{
public class PrintingConfig<TOwner>

This comment was marked as resolved.

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