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

Allow Time::Span to be divided by Time::Span #3749

Merged
merged 1 commit into from Dec 21, 2016

Conversation

RX14
Copy link
Contributor

@RX14 RX14 commented Dec 21, 2016

Dividing a time span by a time span gives a ratio. This is useful for example for working out the number of periods in a larger time (e.g. time_span / 15.seconds).

@asterite
Copy link
Member

@RX14 Nice! Would you mind adding a spec?

@RX14 RX14 changed the title Add Time::Span division by itself Allow Time::Span to be divided by Time::Span Dec 21, 2016
@RX14
Copy link
Contributor Author

RX14 commented Dec 21, 2016

Updated

@@ -207,6 +207,14 @@ describe Time::Span do
# TODO check overflow
end

it "divides by another Time::Span" do
ratio = 20.minutes / 15.seconds
ratio.should eq(80.0)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ratio.should eq(80.seconds)?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is fine. Dividing a time unit by a time unit gives something that has no unit.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's 80.0 because there are exactly 80 15 second intervals in 20 minutes. Adding a unit wouldn't make sense at all.

@asterite asterite added this to the 0.20.2 milestone Dec 21, 2016
@asterite asterite merged commit 858c600 into crystal-lang:master Dec 21, 2016
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