Functionalities to convert date time (in PT format) in total minutes/seconds #26
Conversation
…seconds. eg, when time_str = PT1M60S it will be either 2 mins OR 120 seconds in total. time_in_minutes(time_str) OR time_in_seconds(time_str) will give these result * This may stretch up to hours/days/months even years When a user inserts some random time interval while creating/updating a task (say in minutes; (65)) ,then during retrieving trigger's information, windows returns PT1M5S. Hence, he must be able to check this result as 65 mins again. Signed-off-by: Nimesh Patni <nimesh.patni@msystechnologies.com>
| module Windows | ||
| module TimeCalcHelper | ||
|
|
||
| DAY_OF_MONTH = [0, 31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31] |
There was a problem hiding this comment.
do we use the 0 index here? that feels like kind of a hack. Could we at least make that value nil and add a comment here as to why there are 13 months?
There was a problem hiding this comment.
We don't use 0th index, but the structure of this array is maintained in a way that it returns actual no of days in a month (directly with month number), without any manipulation
eg, JAN(month No: 1) => DAY_OF_MONTH[1] => 31
Usage: @#L8:
(month == 2 && is_leap_year?(year)) ? 29 : DAY_OF_MONTH[month]
Also, I've kept this' 0' purposely. Keeping a 'nil' or 'a blank string' at first place could be prone to exceptions in calculations and hence it may require a conversion (to_i), which is an overhead.(Please let me know your views)
Have updated PR with comment in code explaining its significance.
Thank you
|
|
||
| # Basic time variables | ||
| future_year = curr_time.year + dt_tm_hash[:year].to_i | ||
| future_mth = curr_time.month + dt_tm_hash[:month].to_i |
There was a problem hiding this comment.
can we call this future_month instead of future_mth since we use month in full everywhere else?
There was a problem hiding this comment.
Have updated the variable name. Thank you
Signed-off-by: Nimesh Patni <nimesh.patni@msystechnologies.com>
Signed-off-by: Nimesh Patni <nimesh.patni@msystechnologies.com>
| # Array with a 0 is defined to give actual result without | ||
| # any manipulation. eg, DAY_OF_MONTH[1] = 31 | ||
| # 0(NUMBER) is kept to avoid exceptions during calculations | ||
| DAY_OF_MONTH = [0, 31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31] |
There was a problem hiding this comment.
probably rename this to DAYS_IN_A_MONTH?
There was a problem hiding this comment.
Renamed the variable as per review comment
Thank you
| DAY_OF_MONTH = [0, 31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31] | ||
|
|
||
| # Returns no of days in a given month of a year | ||
| def day_in_month(month, year) |
There was a problem hiding this comment.
And this to get_days_in_a_month for clarity?
There was a problem hiding this comment.
As per ruby best practices, we should generally avoid using 'get_' and 'set_' in method names. It is a trivial technique that we often use in Java/C/C++.(Got this idea while working with Rubocop). Few reasons are listed here
Still, If it is needed for better understanding, will update the code accordingly.
Kindly let us know your views about this
Thank you
|
Working on rspecs for this module. Thanks |
Nimesh-Msys
changed the title
- Minor Fix in time_calc_helper
Nimesh-Msys
changed the title
For an example when time_str = 'PT1M60S' it will be either '2 mins' OR '120 seconds' in total.
time_in_minutes(time_str)ORtime_in_seconds(time_str)will give above results respectively.Significance: When a user inserts some random time interval while creating/updating a task (say in minutes; (65)) ,then during retrieving trigger's information, windows returns PT1M5S. Hence, he must be able to check this result as 65 mins again.
Signed-off-by: Nimesh Patni nimesh.patni@msystechnologies.com