Skip to content

Return DateTime objects instead of strings from temporal methods#10

Open
adammck wants to merge 5 commits into
kennyma:masterfrom
adammck:datetimes_not_strings
Open

Return DateTime objects instead of strings from temporal methods#10
adammck wants to merge 5 commits into
kennyma:masterfrom
adammck:datetimes_not_strings

Conversation

@adammck
Copy link
Copy Markdown
Contributor

@adammck adammck commented May 10, 2012

This is a fairly major change, which breaks backwards compatibility (albeit in an easily-fixable way). So if it seems useful, please check it carefully before merging.

Currently, the time-related accessors like FitnessActivitiesFeed::Item#start_time return strings straight from RunKeeper. They look something like "Thu, 15 Sep 2011 13:28:59". This is fine for displaying as-is, but not a lot of use for anything else. This patch intercepts the values before they're stored (in the Model#populate_from_hash! method), and coerces them into DateTime objects.

The implementation is a little more complex than it needs to be, because I've tried to model it after Hashie::Extensions::Coercion, so we can easily replace this minimal solution with theirs, once a stable release is available. Faraday already uses Hashie, so it isn't a new dependency.

Coercion looks like (from weight_feed.rb):

hash_attr_accessor :timestamp, :weight, :uri # etc
coerce_key :timestamp, HealthGraph::DateTime

The HealthGraph::DateTime class is just a wrapper around ::DateTime which knows how to parse RunKeeper-style timestamps. Any class with a coerce method can be used.

adammck added 5 commits May 10, 2012 18:23
This is (obviously) very similar to the previous commit. I'm just going
to get all of the timestamps working, then refactor this into a helper.
This is a modified reimplementation of Hashie::Extensions::Coercion.
I used the same coerce_key API, so when Hashie 2.0 comes around, this
helper can be removed.

See: https://github.com/intridea/hashie/blob/master/lib/hashie/extensions/coercion.rb
naveed-ahmad pushed a commit to naveed-ahmad/health_graph that referenced this pull request Nov 3, 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