Skip to content

save-load#23

Merged
Bronuh merged 16 commits intomasterfrom
save-load
Jan 24, 2026
Merged

save-load#23
Bronuh merged 16 commits intomasterfrom
save-load

Conversation

@Bronuh
Copy link
Member

@Bronuh Bronuh commented Jan 24, 2026

Add Exposables save/load system and some improvements for KludgeTests

@Bronuh Bronuh requested a review from KeyJ148 January 24, 2026 16:27
object result = null;

// Next bug: this thing can't find the default constructor
//result = Activator.CreateInstance(type, ctorArgs);
Copy link
Member

Choose a reason for hiding this comment

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

Точно надо оставлять этот кусок в коде, мб в задачу по доработке вынести или как-то ещё?

Copy link
Member Author

Choose a reason for hiding this comment

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

Это памятка на будущее, баг пофикшен, просто мне надо будет потом вписать эту херню "в методичку". Разве что TODO сюда бахнуть

Copy link
Member

Choose a reason for hiding this comment

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

Ну либо TODO, либо в идеале в задачу по написанию методички)

/// }
/// </code>
/// </summary>
/// <param name="container"></param>
Copy link
Member

Choose a reason for hiding this comment

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

Мб лучше добавить описание параметра, или убрать совсем блок param?

/// Сохраняет или читает значение
/// </summary>
/// <param name="value">Значение</param>
/// <param name="label"></param>
Copy link
Member

Choose a reason for hiding this comment

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

Аналогично, пустой param

/// <param name="value">Значение</param>
/// <param name="label"></param>
/// <param name="defaultValue">Значение по умолчанию. Никуда не сохраняется.</param>
/// <typeparam name="TValue"></typeparam>
Copy link
Member

Choose a reason for hiding this comment

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

Пустой typeparam

/// Сохраняет или читает сложный объект (IExposable)
/// </summary>
/// <param name="value">Объект</param>
/// <param name="label"></param>
Copy link
Member

Choose a reason for hiding this comment

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

И здесь

ExposeAs exposeValueAs = ExposeAs.Undefined)
{
if (exposeKeyAs == ExposeAs.Undefined)
exposeKeyAs = ResolveExpositionType(typeof(TKey));
Copy link
Member

Choose a reason for hiding this comment

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

Я максимально за codestyle с обязательными скобками для if-ов
Можно делать исключение, но только если там нет переноса строки (т.е. if и команда на одной строке)
Вкусовщина, но почти все современные code-guide за скобки


if (State is ContainerState.ScanReferences)
{
if (dictionary is null)
Copy link
Member

Choose a reason for hiding this comment

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

Вот тут вот как раз хороший пример, где их можно было бы не ставить)
Имхо


if (State is ContainerState.RefsResolving)
{
if (list is null)
Copy link
Member

Choose a reason for hiding this comment

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

Аналогчино, бахни в одну строку лучше

if (State is ContainerState.ScanReferences)
{
if (value is null)
return;
Copy link
Member

Choose a reason for hiding this comment

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

И тут


public class FieldAccessor : IMemberAccessor
{
private static ILogger _log = LogFactory.GetForStatic<FieldAccessor>();
Copy link
Member

Choose a reason for hiding this comment

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

А почему не через DI внедрение?
Или чтобы память сэкономить?

Как у нас вообще принято? Стараться через DI, или нет?
Я просто везде через Di делал)

Copy link
Member Author

Choose a reason for hiding this comment

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

Этот тип сам по себе обеспечивает работу DI на его базовом уровне, да и по отдельному логгеру на каждую ссылку на поле/свойство бахать смысла не вижу

@Bronuh Bronuh merged commit 4747deb into master Jan 24, 2026
1 check passed
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