Basic list implementation #191
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
joshsmith
reviewed
Jan 16, 2017
| @@ -44,6 +42,8 @@ defmodule Stripe.Converter do | |||
|
|
|||
| # converts plain maps | |||
| defp convert_value(value) when is_map(value), do: convert_map(value) | |||
| # converts lists | |||
There was a problem hiding this comment.
Maybe a space above this.
| A list is a general-purpose Stripe object which holds one or more | ||
| other Stripe objects in it's "data" property. | ||
|
|
||
| In its current iteration it simpl allows serializing into a properly |
| Work with Stripe list objects. | ||
|
|
||
| A list is a general-purpose Stripe object which holds one or more | ||
| other Stripe objects in it's "data" property. |
|
|
||
| In future iterations, it should: | ||
| * Support multiple types of objects in its collection | ||
| * Support fetching the next set of objects (pagination) |
There was a problem hiding this comment.
This - is preferable over *
| @@ -0,0 +1,19 @@ | |||
| defmodule Stripe.List do | |||
There was a problem hiding this comment.
What's the thinking behind not using just a map instead of a struct?
There was a problem hiding this comment.
Mostly because we will want to be able to fetch more pages eventually. This will probably be a function of this struct.
There was a problem hiding this comment.
Also, stripe considers it an object (it has an "object" : "list") property, unlike, for example, legal_entity or dob which do not have such properties.
There was a problem hiding this comment.
Okay, I think Stripe considering it an object is good justification.
begedin
force-pushed
the
188-basic-list-implementation
branch
2 times, most recently
from
da043a5
to
0a2f16e
Compare
begedin
force-pushed
the
188-basic-list-implementation
branch
from
0a2f16e
to
13d755b
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #188
This is a basic implementation of a list module. It allows us to serialize list objects.
It still lacks support of pagination and fetching the next page of objects if there are multiple, but that's more advanced and should probably be handled separately.
It should also support having lists whose elements are not all of the same type, but due to lack of supporting all stripe objects, I cannot find a use case that would work as a test.