Skip to content

Fix not working relation list#13

Merged
iluvadev merged 4 commits intoiluvadev:mainfrom
mtaku3:dev-2
Jan 13, 2023
Merged

Fix not working relation list#13
iluvadev merged 4 commits intoiluvadev:mainfrom
mtaku3:dev-2

Conversation

@mtaku3
Copy link
Copy Markdown
Collaborator

@mtaku3 mtaku3 commented Jan 13, 2023

Hi, when I was using ORM for my project, I found out that list of relation and enum isn't working.
Specifically, CollectionList and EnumList.

There are two reasons that it is not working.

  1. JsonSerializer doesn't serialize readonly property.
    CollectionList and EnumList are readonly(setter is private, not public) in auto-generated code. JsonSerializer skips these properties. To include, we need to add [JsonInclude] attribute to property explicitly.
  2. FieldBasicList doesn't have implementation of object? IBasicList.Add(object? element)
    Looks like setter of these properties calls
    Set => UpdateWith => Add
    But FieldBasicList class doesn't have implementation of object? IBasicList.Add(object? element). (returning null)
    I added implementation using FieldBasicList.Add. So that element will be added to the inner list properly.
    (I added implementation of object? IBasicList.Remove(object? element) as well because I think it is necessary.)

Also, current implementation of

        bool IBasicList.Contains(object? element)
            => false;

in FieldBasicList class maybe wrong?

I don't fully understand the code so maybe some part are wrong. Sorry for that.

Copy link
Copy Markdown
Owner

@iluvadev iluvadev left a comment

Choose a reason for hiding this comment

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

Thanks again! Good aportation

  • private setters in the lists was an "improvement" to protect the object list, but i was too optimistic and rushed to push it without testing enough. Of course, private getters/setters can not be serialized/deserialized without forcing. Sorry!
  • The other changes are good. Let me explain what i was thinking when i coded that:
    • There are two functions Remove and Add, that had something like:
public T? Add(T? element)
{
  // The function Add logic
}

object? IBasicList.Add(object? element)
  => null; // Why??

The function object? IBasicList.Add(object? element) is as private (yes, "as private"):

  • The function is not visible outside the object itself: it could do nothing... what i did 👎🏼
  • But the object implements IBasicList
    • The function is visible when the object is used as IBasicList

In principle the object should not be used as the generic interface... But... why? And... the code is suspicious, unclear, unpredictable, and ultimately a potential source of bugs

Good aportation. Thanks a lot. 🥇
I will review the code like this to fix it

@iluvadev
Copy link
Copy Markdown
Owner

I changed

bool IBasicList.Contains(object? element)
    => false;

To

bool IBasicList.Contains(object? element)
    => element is T item && Contains(item);

Thanks @mtaku3

@iluvadev iluvadev merged commit 7e94b25 into iluvadev:main Jan 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants