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

Rework resource types #39

Merged
merged 3 commits into from May 31, 2016
Merged

Rework resource types #39

merged 3 commits into from May 31, 2016

Conversation

jgraichen
Copy link
Owner

  • Drop allow_nil and allow_blank

    allow_nil should be replaced by validations and allow_blank broke
    much code with less to none use.

  • All type handle nil, empty and blank strings
    Primitive types cast to false, 0 or 0.0 while other types cast to nil (e.g. DateTime, etc)

Type casting is still strict, casting "123a" to integer will raise an
error and not return 123 like #to_i. List and dict types support more
type cohesion methods: #serializable_hash, #to_hash, #to_h on dict
and #to_ary, #to_a on list.

/cc @mswart @cwillems

@jgraichen
Copy link
Owner Author

Open questions are:

  • Should blank strings convert to nil on boolean types?
  • Should list type convert everything non-arryish to an array, like Array(whatever)?

* Drop `:allow_nil` and `:allow_blank`

  `:allow_nil` should be replaced by validations and `:allow_blank` broke
  much code with less to none use

* All type handle nil, empty and blank strings

  Primitive types cast to false, 0 or 0.0 while other types cast to nil (e.g. DateTime, etc)

Type casting is still strict, casting "123a" to integer will raise an
error and not return 123 like `#to_i`. List and dict types support more
type cohesion methods: `#serializable_hash`, `#to_hash`, `#to_h` on dict
and `#to_ary`, `#to_a` on list.
@mswart
Copy link
Collaborator

mswart commented May 29, 2016

I prefer interpret blank string as nil - as it is the best representation for use the default / not set.

Converting everything into arrays is probably the better approach: it simplifies to-array conversations. With non-verified array elements I do not see much advantage on enforcing that we get an array itself.

* Boolean type: Cast blank strings to nil
* List type: Cast non-arrayish object to an array using `Array(value)`
@jgraichen
Copy link
Owner Author

jgraichen commented May 29, 2016

One other question appeared:

Acfs uses a whitelist of values that are true when casting boolean. This is somewhat contradictory to how Ruby interprets bool values (everything is true, except falsy things) and how ActiveRecord or ActiveModel do it, using a list of falsy values (0, "false", "off", ...).

Do we want to change that too?

@cwillems
Copy link
Collaborator

Acfs uses a whitelist of values that are true when casting boolean. This is somewhat contradictory to how Ruby interprets bool values

I'd actually go for ActiveRecord-like behavior.

@jgraichen
Copy link
Owner Author

Would go for release when CI passes unless someone objects.

@jgraichen jgraichen merged commit e4aa0d8 into master May 31, 2016
@jgraichen jgraichen deleted the types branch May 31, 2016 15:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants