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

Activates subtraction operator with right side tod object #35

Conversation

hirokishirai
Copy link
Contributor

Regarding addition and subtraction operators, I wanted functions like following.

right_side = Tod::TimeOfDay.new(10,0,0)
# => #<Tod::TimeOfDay:0x007fe98a5a8c70 @hour=10, @minute=0, @second=0, @second_of_day=36000>
left_side = Tod::TimeOfDay.new(12,0,30)
# => #<Tod::TimeOfDay:0x007fe98a55b178 @hour=12, @minute=0, @second=30, @second_of_day=43230>
result = right_side - left_side
# => #<Tod::TimeOfDay:0x007fe98a4db1a8 @hour=21, @minute=59, @second=30, @second_of_day=79170>

This PR enables addition and subtraction when Tod::TimeOfDay Object is on the operators right side.

@jackc
Copy link
Owner

jackc commented Apr 30, 2016

I'm not convinced that adding times makes sense. What does it mean to add 3AM to 8AM?

I think we should follow the precedent of the Ruby standard Time object. Subtracting times is okay, adding them is not.

Limit this PR to subtraction and I'll merge it.

@hirokishirai
Copy link
Contributor Author

I see. I understand.
I'll remove the changed codes of addition.

@hirokishirai hirokishirai changed the title Activates addition and subtraction operators with right side tod object Activates subtraction operators with right side tod object May 1, 2016
@hirokishirai hirokishirai changed the title Activates subtraction operators with right side tod object Activates subtraction operator with right side tod object May 1, 2016
@jackc jackc merged commit b96feda into jackc:master May 6, 2016
@jackc
Copy link
Owner

jackc commented May 6, 2016

Thanks!

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.

None yet

2 participants