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

Type Time and NaiveTime differently #30

Closed
jrstrunk opened this issue May 18, 2024 · 5 comments
Closed

Type Time and NaiveTime differently #30

jrstrunk opened this issue May 18, 2024 · 5 comments

Comments

@jrstrunk
Copy link

Hey everyone, just wanted to start a discussion. Since Gleam has a really nice type system and as a language heavily encourages you to use types to reduce the possibility of bugs or crashes, I thought it would really help reduce time related bugs if Time and NaiveTime were different types. I know a big downside is that it would add a lot more code to this package and probably bump birl up a major version, but it will really make sure that when people do comparisons and conversions between times, they know how time offsets will be handled. For example, if I wanted to compare two time values, say 2019-01-02T14:00:00.000 and 2019-01-02T14:00:00.000-04:00, it is not obvious to me how the compare function will behave in this case. Maybe only the naive times are compared, maybe they are both converted to UTC and compared, I cannot know until I run some tests. If instead I was forced to convert them both to the same time type myself, then I could be sure how they are being compared: I could give the first an offset and compare it to the second with the compare function. Or I could make the second naive by calling to_naive (and rename the current func to to_naive_string) and compare them with a compare_naive function. This gives me, the application implementer, explicit control over how I deal with time offsets and zones, instead of assuming the birl package will handle them the way I want every time.

@jrstrunk
Copy link
Author

For another example, without running tests or knowing the implementation, can anyone guess what this will return:

  let assert Ok(t) = birl.parse("2019-01-01T14:00:00.003")
  birl.get_offset(t)
  |> io.debug

The result is "-01:00". I wouldn't have guessed that myself, but maybe we shouldn't be able to pass a NaiveTime into a function like get_offset. Making them two types would prevent this. Love the package, just want it to be the best it can be!

@massivefermion
Copy link
Owner

massivefermion commented May 23, 2024

As has been mentioned in #29, this is a bug in parse, it should return an error for that input. But your suggestion about having a separate type for naive values makes sense, I think Eilxir's standard library has that. I'll definitely consider this for version 2.
Thanks

@jrstrunk
Copy link
Author

Hey! This issue and the want for easy decision-making around the current time vs a target time are what prompted me to create tempo. The standard time library is being worked on as you know, so that might change what the future of Birl looks like. I think it'll always make sense to have external time libraries, so I'm sure Birl will still serve a purpose in the long run. But I think my original wants for this issue have been fulfilled externally, so I am inclined to close it unless you object.

Also, I have to say Birl has provided a great resource to the community being the first time package by far, and the name is fantastic!

@massivefermion
Copy link
Owner

Thanks for the kind words @jrstrunk
But to be honest, given the weird state of my life, I don't think I can keep up with the gleam community anymore which is sad. For some time I was hoping at some point I would regain my enthusiasm and motivation towards my occasional gleam contributions or packages, but it hasn't happened yet and I'm not sure if it's going to happen or not at this point. We'll see.

@jrstrunk
Copy link
Author

jrstrunk commented Jan 6, 2025

I totally understand, maintaining open-source packages is a lot of hard work for little gain. I hope you do regain motivation at some point, we all learn from each other and are better together!

Since Birl has seen a lot of adoption, I might encourage you to find another member of the community to give co-access to this repo and hex package in case dependencies need to be updated or something and you are away. Louis is an easy pick, but you might know someone else you'd like to onboard. Completely up to you, just thinking long term!

Anyway thanks again for your contributions, I'll close this issue now. Cheers!

@jrstrunk jrstrunk closed this as completed Jan 6, 2025
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

No branches or pull requests

2 participants