Coerce collections of parseable types #1711
Conversation
| # @return [Array,Set] the coerced result. May be an +Array+ or a | ||
| # +Set+ depending on the setting given to the constructor | ||
| def call(value) | ||
| coerced = value.map { |item| super(item) } |
There was a problem hiding this comment.
I'm aware this will fail if value is a string but I wasn't sure of the best thing to do about it. Maybe Array.wrap?
There was a problem hiding this comment.
To allow for a single item to be passed in as such (without the array wrapping/declaration) is a convenience, but could be improper. I suppose it depends on how strict Grape wants to be.
There was a problem hiding this comment.
I would deal with that scenario later with a proper spec.
| # @return [Array,Set] the coerced result. May be an +Array+ or a | ||
| # +Set+ depending on the setting given to the constructor | ||
| def call(value) | ||
| coerced = value.map { |item| super(item) } |
There was a problem hiding this comment.
I would deal with that scenario later with a proper spec.
dslh
force-pushed
the
custom-type-array-coercion
branch
from
0d8be56 to
cae9274
Compare
dslh
force-pushed
the
custom-type-array-coercion
branch
from
cae9274 to
3ed8bbe
Compare
|
Readme and changelog are updated. It didn't seem like there was much to be said about the functionality added in this PR; once you have custom types, wrapping them in an array or a set is something that should "just work". But I did notice that the existence of the |
|
Perfect. |
Hi @dblock and @javmorin!
This implements support for "
type: Array[type that responds to .parse]", as discussed in #1700.CustomTypeCoerceris leveraged, so the type may also implementcoerced?orparsed?to implement custom type-checking (or alsocall, which I admit seems a bit weird in this instance).Supporting
type: Array[Virtus::Attribute subclass]should also be possible but I think it needs some different plumbing to make it work. From the discussion it wasn't clear to me that there's anyone who is specifically looking for it, or if this PR would make it redundant. Let me know if there's any interest and I can take a look at it.Supports
Setdeclarations as well asArray.Hashdidn't make much sense to me in this context but if anyone has a usage example in mind I could take a look.A couple more tests could be appropriate, I'll also get the readme and changelog filled in when I can, but now seemed like a good time to get get some feedback. Is there anything that could be changed or added?