-
Notifications
You must be signed in to change notification settings - Fork 7
Add datetime conversion functionality with tests #15
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
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems, there is some problem with open-api-spec.yml. because it just failed to render OpenAPI yaml in editors.
You can check from online editor.
https://editor.swagger.io/
Error:
An error occurred during specification rendering
Error: Unable to parse specification: mapping values are not allowed here in 'reader', line 248, column 65: ... to be converted to words (format: YYYY-MM-DD) ^
|
||
// ConvertDate converts a date to words in Azerbaijani | ||
// Example: "2024-04-24" -> "iyirmi dörd aprel iki min iyirmi dörd" | ||
func ConvertDate(dateStr string) (string, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest changing the method signature to accept a Go type (such as time.Time or time.Duration) instead of a hardcoded string. This would avoid forcing clients to manually follow a specific string pattern and make the API more type-safe. What do you think?
|
||
// ConvertTime converts a time to words in Azerbaijani | ||
// Example: "14:30" -> "on dörd otuz" | ||
func ConvertTime(timeStr string) (string, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we can change the method to accept a time.Duration instead of a string, so that clients are required to pass a type-safe input. This would remove the need for manual parsing or validation inside the method. What do you think?
} | ||
|
||
// ConvertTime converts a time to words in Azerbaijani | ||
// Example: "14:30" -> "on dörd otuz" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently, it only converts hours and minutes into words. How should we handle durations that also include seconds (and optionally milliseconds)?
For example:
14:30:12 → 14 saat 30 deqiqe 12 saniye
|
||
minute, err := strconv.Atoi(parts[1]) | ||
if err != nil { | ||
return "", fmt.Errorf("invalid minute: %v", err) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of losing context when returning errors, Go provides a built-in way to wrap errors using %w in fmt.Errorf()
. This preserves the original error and adds meaningful context for easier tracing and debugging.
✨ Why wrap errors?
✅ Preserve the root cause while adding helpful context
✅ Enable errors.Is()
and errors.Unwrap()
for better error handling
|
||
// ConvertDateTime converts both date and time to words in Azerbaijani | ||
// Example: "2024-04-24 14:30" -> "iyirmi dörd aprel iki min iyirmi dörd, on dörd otuz" | ||
func ConvertDateTime(datetimeStr string) (string, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of accepting datetime as a string
and parsing it later, can we update the function to accept a Go native type like time.Time
?
This would make the API safer, avoid unnecessary parsing, and better express the intent at the type level.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@devrufat Thank you for the great contribution! 🚀
I’ve left a few comments — if anything is unclear, feel free to ask or share your thoughts.
This PR adds datetime conversion functionality to the aznum2words library. The changes include:
Features added:
New files:
datetime/constants.go
: Contains Azerbaijani language constants for dates and timesdatetime/converter.go
: Main conversion logic for dates and timesdatetime/converter_test.go
: Comprehensive test suite for the datetime convertercmd/aznum2words-webapp/api/converterapi/converter-api_test.go
: API tests for datetime conversionTests:
This enhancement will allow users to convert dates and times to Azerbaijani words, making the library more comprehensive for text-to-speech and document processing applications.