Using only months and days drops weeks and produces surprising results
Opened this issue · 8 comments
Hello everyone!
I'm trying to use only months and days, but the value I'm getting is not right. If I set the date to 9/28 I get "1 month and 2 days" instead I should get "1 month and 9 days". So I guess it's omitting the week and is not combining the week into days. Am I doing something wrong or is this an issue? Thanks!
PS: here's the code
distance_of_time_in_words(date_a, date_b, false, :only => [:months, :days])
This is working as intended (https://github.com/radar/dotiw#only), but maybe you can come up with an implementation of the :accumulate_on
option that allows you to accumulate on multiple measurements.
2 years after this change I bumped my version of the Gem from 3.01 to the latest, and there's been a bug across my application as a result.
The application used only: [:months,:days]
in many places, and so I'd previously get output like "1 month and 13 days" which I would argue in many contexts is much more readable and easily understandable than "1 month and 1 week and 6 days".
My point is that this issue raised by @jmperez127 is not actually a new feature request. The introduction of the weeks concept has resulted in a change in behavior. And, since it's by far the fastest way to address it, I'm likely to just add :weeks
to my options, even though I think it results in an inferior output for readability.
Anyway, I greatly appreciate anyone that's helped build and maintain this Gem. I don't mean to come off like this is a huge problem or give anyone a hard time. Just that for posterity this issue is not exactly a new feature request, so much as a change in behavior from the way the Gem had been behaving.
I'll take a look at the :accumulate_on
code to see if it's within my somewhat limited skills to try and help with this, but there's a good chance I would add more hassle than help.
While this works as expected I personally do think it's surprising. Does anyone have a strong use-case of only
that would drop intermediate results? Maybe that shouldn't be allowed and only:
be refactored into truncate_at:
or something like that?
@dblock while it might be complex, a nifty way to do it would be to drop results SMALLER than the smallest value in the only
array. For example, let's say you specify:
only: [:years, :months, :days]
Here, the smallest value in the array is days. Therefore anything for hours, minutes, and seconds is dropped. However, weeks is not smaller than the smallest value in the array, so rather than being dropped, it gets "downward converted" to the next smallest specified value.
The more I think about this as I type it, the more I suspect it would be absurdly complex and not worth the effort. But since I've already typed it up, I might as well post it and perhaps it will help you brainstorm!
I think what you propose is an interesting idea! I don't think it's too bad to implement though if you want to give it a shot:
- Build the hash normally, you have years, weeks, days, months, minutes, seconds ...
- From highest to lowest: if the value is excluded (not in
:only
or included inexcept:
) convert down to the next value, and add to it - Truncate at the lowest.
I'd start by writing a bunch of specs :)
Hi @dblock I hope to find the time at some point to give this a shot, but I'm also not that experienced of a developer, have never really contributed to a gem like this, and might take me a while to get it acceptably implemented. But hopefully I can work on it some day.
Hi @dblock I hope to find the time at some point to give this a shot, but I'm also not that experienced of a developer, have never really contributed to a gem like this, and might take me a while to get it acceptably implemented. But hopefully I can work on it some day.
Cool. I would start by writing tests of what you expect the output to be, and that alone is valuable.
If we could make _display_time_in_words
a public method, we could use the raw hash, and further calculate any value we want
[1] pry(main)> hash = DOTIW::Methods.distance_of_time_in_words_hash(Time.now, 72.days.ago, except: [:minutes, :hours])
=> {:years=>0, :months=>2, :weeks=>1, :days=>4, :minutes=>59, :seconds=>59}
[2] pry(main)> hash = hash.merge(days: hash[:days] + (hash.delete(:weeks) || 0) * 7)
=> {:years=>0, :months=>2, :days=>11, :minutes=>59, :seconds=>59}
[3] pry(main)> DOTIW::Methods.send(:_display_time_in_words, hash)
=> "2 months, 11 days and 59 minutes"