Skip to content

[WIP] Time class #525

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

Closed
wants to merge 3 commits into from
Closed

[WIP] Time class #525

wants to merge 3 commits into from

Conversation

hachi8833
Copy link
Member

Implemented a minimum Time class in vm/time.go and lib/time.gb:

» Time.new
#» 2017-11-18 18:51:48.749644 +0900 JST m=+2.760456980
» a = Time.new('2017-05-30')
#» 2017-05-30 00:00:00 +0000 UTC
» a = Time.new('2017-05-30 17:30')
#» 2017-05-30 17:30:00 +0000 UTC
» a = Time.new('2017-05-30 17:30 JST')
#» 2017-05-30 17:30:00 +0900 JST
» a.parse("1999-07-01")
#» 1999-07-01 00:00:00 +0000 UTC
» a.parse("1999-07-01").to_s
#» 1999-07-01 00:00:00 +0000 UTC

I think we need Time class to write loggers or like that.


I'd like to ask @st0012 and other contributors how this Time class should be extended because this is a language design matter and this should meet st0012's philosophy.

My rough memorandom:

  • Avoid creating Date or DateTime class: they are pretty confusing in Ruby. I guess aggregating any methods for date/time/timezone into this Time class might be good.
  • Create a Duration class to represent time durations: for example, the difference between two instances of Time spawns an instance of Duration class. And adding helper methods like 1.month or 2.days as ActiveSupport does.
  • Make Time and Duration classes standard libs that can be loaded via require to be optional. Especially, Duration class would mutate numeric classes.
  • Get the default timezone from ENV when performing Time#new.

How about this?

Add tests for Time class
Add tests for Time#parse
@ghost ghost assigned hachi8833 Nov 18, 2017
@ghost ghost added the in progress label Nov 18, 2017
@64kramsystem
Copy link
Member

Avoid creating Date or DateTime class: they are pretty confusing in Ruby. I guess aggregating any methods for date/time/timezone into this Time class might be good.

While Time and Datetime have no significant difference nowadays, Date definitely has its own, separate domain, as it's meant to represent dates regardless of their time zones, which is semantically different from the concept of time (intended as timestamp with time zone).

@64kramsystem
Copy link
Member

Create a Duration class to represent time durations: for example, the difference between two instances of Time spawns an instance of Duration class. And adding helper methods like 1.month or 2.days as ActiveSupport does.

This is a complex subject, for a few reasons, and I think it should be entirely separate:

  1. what is 1.month? 28, 29, 30 or 31 days?
  2. what's its unit? seconds or fractions of seconds? this is crucial when performing operations, as it will affect the resulting data type.
  3. 1.seconds is monkeypatching; is something that will be accepted in Goby? there is a large amount of negativity about it - but admittedly, it's also convenient. Golang in fact, works this around by using the format time.Duration type, plus N * time.<time_type> representations
  4. as consequence of 3., should Goby introduce refinements? if 1.seconds should be accepted, likely, it should be implemented as refinement, in order to avoid monkeypatching introduction

and likely, there are several other problems. I think this should be split into another issue.

@st0012
Copy link
Member

st0012 commented Nov 18, 2017

@saveriomiroddi @hachi8833 I think if we make seconds or months as Integer's APIs, it won't be monkey-patching. However, introducing these APIs in Integer class is somehow inappropriate. So I think we might spend some time discussing this.

@hachi8833
Copy link
Member Author

Thank you for checking! I'd hold on for a while until discussions are finished.

@st0012
Copy link
Member

st0012 commented Nov 18, 2017

And about monkey-patching, I don't want users to do that but we've used it in some standard libraries. So we need a mechanism to lock monkey-patching after the VM is completely initialized. This also requires further discussion.

@st0012
Copy link
Member

st0012 commented Dec 16, 2017

@hachi8833 Can you remind me what stop us from finishing this?

@hachi8833
Copy link
Member Author

Closing it to re-implementation later

@hachi8833 hachi8833 closed this Apr 15, 2018
@ghost ghost removed the in progress label Apr 15, 2018
@st0012 st0012 deleted the time_class branch September 30, 2018 14:40
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.

3 participants