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

#36 Added arrayindex of the object to modelObjectPath in ModelGraphCache #37

Merged
merged 1 commit into from
Nov 25, 2020

Conversation

yveslaurentcreton
Copy link

… iteration (ModelGraphCache)

I've added the arrayIndex of the object to modelObjectPath so that the next iteration this can be used in the cache to link the child objects to the correct arrayIndex of the property

… iteration (ModelGraphCache)

I've added the arrayIndex of the object to modelObjectPath so that the next iteration this can be used in the cache to link the child objects to the correct arrayIndex of the property
@ryanelian
Copy link
Owner

Sorry for the late reply and thanks for the PR.

I remember leaving this behavior out from the object graph cache to conserve memory:

// Constructing model object path here allows capturing the same array objects without the index!
if (string.IsNullOrEmpty(modelObjectPath))
{
modelObjectPath = propertyName;
}
else
{
modelObjectPath += "." + propertyName;
}

If the array has 1000000 items, I don't want the cache to record all 1000000 objects, but just the 1 parent array object to conserve memory. That

And for that, this PR is rejected, sorry.

@ryanelian ryanelian closed this Nov 13, 2020
@yveslaurentcreton
Copy link
Author

@ryanelian I've done this fix and this pull request to fix #36. When debugging, you see that the modelObjectPath isn't really the model object path then. So, caching a value int the wrong path results in wrong behavior.

@ryanelian
Copy link
Owner

Interesting, so that PR is a fix to an existing problem. Let me take a look at it (soon).

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.

None yet

2 participants