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

Add @time.fixed_zone_unvalidated #98

Closed
wants to merge 2 commits into from

Conversation

gmlewis
Copy link

@gmlewis gmlewis commented Jan 23, 2025

This function is needed so that a default @time.Zone can be passed to a function (other than utc_zone).

Another name for this could be fixed_zone_or_utc if fixed_zone_unvalidated is not wanted.

It fixed problems like this:

need-unvalidated-time-zone-2025-01-23_09-19-46

Signed-off-by: Glenn Lewis <[email protected]>
@peter-jerry-ye
Copy link
Collaborator

Sorry but I didn't fully understand the context of this PR.

Is the unvalidated needed for performance issue or does the current validation reject any inputs that you find valid? And what does 'default @time.Zone' should be?

@gmlewis
Copy link
Author

gmlewis commented Jan 24, 2025

Sorry but I didn't fully understand the context of this PR.

Is the unvalidated needed for performance issue or does the current validation reject any inputs that you find valid? And what does 'default @time.Zone' should be?

I'm trying to write a function with a default parameter of type @time.Zone.
However, because it has !Error, the compiler will not let me do it.
See screenshot above. Does that make sense?

In other words, I can't write this:

///| Returns the current time.
pub fn get_current_time(
  /// now is optional so we can mock time in tests
  now~ : @wallClock.Datetime = @wallClock.now(),
  zone~ : @time.Zone = @time.fixed_zone!("America/New_York", -5 * 60 * 60)
) -> @time.ZonedDateTime!Error {
  @time.unix!(now.seconds.reinterpret_as_int64(), zone~)
}

However, because @time.fixed_zone!(...) has a ! in it, the compiler won't let me.

Am I doing something wrong?

@peter-jerry-ye
Copy link
Collaborator

How about this:

///| Returns the current time.
pub fn get_current_time(
  /// now is optional so we can mock time in tests
  now~ : @wallClock.Datetime = @wallClock.now(),
  zone? : @time.Zone
) -> @time.ZonedDateTime!Error {
  let zone = zone.or(@time.fixed_zone!("America/New_York", -5 * 60 * 60))
  @time.unix!(now.seconds.reinterpret_as_int64(), zone~)
}

@gmlewis
Copy link
Author

gmlewis commented Jan 24, 2025

Yes, that would work, but now zone must be passed to the function.
Is there any way to call get_current_time() without any parameters and have it default to a particular time zone, yet allow the caller to override the time zone in the function call with a named zone~ parameter?

@peter-jerry-ye
Copy link
Collaborator

peter-jerry-ye commented Jan 24, 2025

uh, a ?: T is an optional parameter. Why do you have to pass a parameter?

Maybe you want to check this: https://docs.moonbitlang.com/en/latest/language/fundamentals.html#optional-arguments

@gmlewis
Copy link
Author

gmlewis commented Jan 24, 2025

Oh, wow, OK. I guess I need to go back and read all the docs again, as I'm forgetting parts of the language.

Thank you, @peter-jerry-ye !

Closing.

@gmlewis gmlewis closed this Jan 24, 2025
@gmlewis gmlewis deleted the add-time-zone-unvalidated branch January 24, 2025 03:35
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.

2 participants