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

Fix for CBL-ios issue 648 #669

Closed
wants to merge 2 commits into from
Closed

Fix for CBL-ios issue 648 #669

wants to merge 2 commits into from

Conversation

24601
Copy link

@24601 24601 commented Apr 20, 2015

This is my fix for #648. Intentionally excluding NSDate in the NSObject subclass check since it is handled best in the final else catch-all case, and NSObject handing of NSDate is not appropriate/does not work. Maybe more elegant way to implement this as there might be other cases (e.g. other classes) that the coe should be letting the final catch-all else handle using ValueConverter handle (I doubt NSDate is the only such case?), but wanted to avoid significantly changing the structure/flow of this code as it is not originally my code and my understanding of it is not good enough and wanted to leave the least trace possible with this fix, I can imagine reworking this in a few ways to handle this, but it would probably change the method and break other cases, so excluding NSDate subclasses in the NSObject else if I added for now...

#648

@24601
Copy link
Author

24601 commented Apr 20, 2015

To add on to my above comments on the nature of excluding certain classes in the NSObject subclass check (which I think is a bit kludgy), perhaps a cleaner way of accomplishing this is to, in the final catch-all else (in both array item and property code paths) to try value converter first and if converter is nil, then do return [super impForGetterOfProperty: property ofClass: propertyClass];, would do a better job of "deferring to ValueConverter" like I mention above than the current implementation, perhaps?

Also make updates to ValueConverter to handle more data types "automatic" in this case....

better handling of the need to defer to ValueConverter and only handle
otherwise-unhandled subclasses of NSObject
@24601
Copy link
Author

24601 commented Apr 20, 2015

Implemented the above fix as it is much cleaner and will resubmit this PR.

@24601 24601 closed this Apr 20, 2015
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.

1 participant