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

Resource collections #39

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
Open

Resource collections #39

wants to merge 12 commits into from

Conversation

jkrumow
Copy link
Contributor

@jkrumow jkrumow commented Jan 25, 2016

Hey Josh,

we have another PR!

We have modified the data model to map all the properties of a relationship onto a resource collection.
So instead of using an NSArray there is now a class JSONAPIResourceCollection which contains all related resources but also stores links to self and related. It also behaves like an NSArray (indexed subscripting, enumeration etc).

Best regards
Julian Krumow

@joshdholtz
Copy link
Owner

Thank you @tarbrain! I assume this is probably a breaking change to the API then? 😇 (Didn't take a close look yet)

@jkrumow
Copy link
Contributor Author

jkrumow commented Jan 25, 2016

Yes, the library user would have to change the relationships on their model objects from NSArray to JSONAPIResourceCollection.

But then it is possible to lazily load a resource graph by requesting a resource with only a shallow relationship node, and later on you can request the content of the collection by its links.

@joshdholtz
Copy link
Owner

@tarbrain Cool cool, thanks! I will keep that in mind. One of my initial thoughts would be to keep the current naming for NSArray and make a new property/function for the JSONAPIResourceCollection as I don't like to introduce breaking changes but I will take a look and let you know my final thoughts 👌

@jkrumow
Copy link
Contributor Author

jkrumow commented Jan 25, 2016

There is an NSMutableArray property called resources inside the JSONAPIResourceCollection class.
It is also publicly accessible.

@joshdholtz
Copy link
Owner

@tarbrain Perfect! That is good to know... That may be alright then ☺️

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.

2 participants