Yalantis / FastEasyMapping

A tool for fast serializing & deserializing of JSON
https://yalantis.com/
Other
551 stars 79 forks source link

Move delegate invocation earlier in method #121

Closed jcavar closed 4 years ago

jcavar commented 4 years ago

This enables https://github.com/Yalantis/FastEasyMapping/issues/119, but I think is generally better approach.

Please merge https://github.com/Yalantis/FastEasyMapping/pull/120 before this.

jcavar commented 4 years ago

Just a note on isInserted. This check was introduced in 29b1d34. Do you maybe remember reasons for that change @dimazen?

dimazen commented 4 years ago

@jcavar honestly, I don't remember exact root of isInserted change. It looks like a bugfix to prevent storing in the Store's cache objects that doesn't come from it, however I don't see any other way to insert anything that wouldn't be returned from the -[FEMManagedObjectStore newObjectForMapping:].

Regarding your change: doesn't look harmful but willMap was intended to be invoked when we precisely know that we're going to run mapping for 100% (which may not be the case under certain conditions).

Let's get back to it after https://github.com/Yalantis/FastEasyMapping/issues/119 clarifying. I just have an idea of adding a new method that will allow user to customise which object should be returns, therefore delegate can run any arbitrary code when needed. This seems to be more semantically correct. What do you think? I just want to clarify your issue and then I believe we'll come up with a final solution :)