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.

Sign up for GitHub

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

Jump to bottom

[Feedback-Required] Making layout serialization async #264

Closed
X39 opened this issue Apr 19, 2021 · 2 comments
Closed

[Feedback-Required] Making layout serialization async #264

X39 opened this issue Apr 19, 2021 · 2 comments

Comments

@X39
Copy link

X39 commented Apr 19, 2021 •
edited
Loading

I recently came across the need to make layout serialization async.

While this was quite an unpleasent experience, i got it to work....
Not, the reason i created this ticket is the following:

I plan on creating a PR that actually provides that capability to "the masses" and look for advice on what exactly to change.

On my journy to making this async, i found numerous of interfaces encapsulated internal, making the approach i chose for doing things async utilize reflection as the required changes where requiring replacing the event-callback system with a method-callback system (and making everything async down to that point.)

To be more precise, this code:

AvalonDock/source/Components/AvalonDock/Layout/Serialization/LayoutSerializer.cs

Lines 64 to 70 in df288d6

foreach (var lcToAttach in layout.Descendents().OfType<ILayoutPreviousContainer>().Where(lc => lc.PreviousContainerId != null))
{
var paneContainerToAttach = layout.Descendents().OfType<ILayoutPaneSerializable>().FirstOrDefault(lps => lps.Id == lcToAttach.PreviousContainerId);
if (paneContainerToAttach == null)
throw new ArgumentException($"Unable to find a pane with id ='{lcToAttach.PreviousContainerId}'");
lcToAttach.PreviousContainer = paneContainerToAttach as ILayoutContainer;
}

is a mess right now since all of it requires internally encapsulated stuff:

AvalonDock/source/Components/AvalonDock/Layout/ILayoutPaneSerializable.cs

Lines 13 to 17 in df288d6

internal interface ILayoutPaneSerializable
{
/// <summary>Gets/sets the unique id for this layout pane.</summary>
string Id { get; set; }
}

AvalonDock/source/Components/AvalonDock/Layout/ILayoutPreviousContainer.cs

Lines 12 to 17 in df288d6

internal interface ILayoutPreviousContainer
{
ILayoutContainer PreviousContainer { get; set; }
string PreviousContainerId { get; set; }
}

To come to an end (TL;DR):

  • Should i split up that method into multiple parts?
  • Make the interfaces public?
  • Something else?

Also regarding an async approach, the question is also wether to utilize a single callback method or something else here too?

Thanks for your time,
X39
@Dirkster99
Copy link
Owner

Hi X39,

it would be great to have an async version for serialization.

Splitting the method into multiple parts sounds good as it could be better testable with unit test and easier to maintain(?).

As far as interfaces go I am not against making them public - maybe this will also help us to identify new functionalities that we can base on this public interfaces?

@bdachev @ryanvs @mgnslndh @amolf-se @eriove
Do you guys have an opinion on the question that is raised in the above comment?

@X39 X39 mentioned this issue Apr 19, 2021
Closed
@X39
Copy link
Author

X39 commented Apr 19, 2021

Created a PR with the changes i propose.

Please leave feedback there.

@X39 X39 closed this as completed Apr 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@X39 @Dirkster99