-
-
Notifications
You must be signed in to change notification settings - Fork 77
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
Added TemporalAdjuster for rounding to nearest part of an hour #196
base: main
Are you sure you want to change the base?
Conversation
I'm happy with the concept that adjusters could be added for use cases like these. However the implementation, as a series of if/else is not desirable. Plus, thus methods are not "nearest", but "truncate". What needs implementing is a single adjuster with a parameter that indicates the amount to truncate to. This can be exposed publicly, eg There might separately be a case for HALF_UP rounding |
Thanks for the feedback Stephen! Could you elaborate on why the methods are truncate instead of nearest? I thought truncate means losing a part of something. Dropping everything below minutes for example. What I am trying to achieve is rounding towards the nearest/closest part of an hour. Given 17:27 the next quarter would be 17:30. Given 17:33 the next quarter again is 17:30. This of often done in payroll or scheduling. |
... why does the implementation advance an entire day, instead of rounding to the hour? You also have so many test cases they become noise (you mostly only need ones around the transition boundaries).
This is often wage theft from employees. Modern timekeeping systems should record actual clock-in/-out times and work from that. |
That is a mistake done on my part. Advancing to the next day should only happen if one rounds from 23:50 to midnight on the next day. Thanks for pointing this out.
In theory you are correct, but unfortunately this is not how its done in practice. I am not a lawyer, but I would not call it theft. For example if I clock in at 7:59 and it gets rounded to 8:00 then yes I lose a minute, but if one clocks out at say 17:40 and the time gets rounded to 17:45 you get five minutes. But again, I am not a lawyer. |
Maybe I misread the code. "nearest" should, IMO, mean the same as HALF_UP rounding. |
Ok, different approach. Thank you for the discussion. I understand that "nearest" was not a good idea. How about calling it rounding and introducing a rounding mode?
Duration in minutes must be a divisor of 60, since there are sixty minutes in an hour and we want to divide the hour into equal parts. The rounding mode is one of UP, DOWN or HALF_UP.
I think with the added rounding mode we can fit several use cases. In scheduling you often want to refer to the next quarter of half an hour. And yes in time keeping - I agree this is controversial - you might need to round HALF_UP. |
How about durations like PT45M (32 × PT45M = PT24H) or PT100S (36 × PT100S = PT1H)? Why would they have to be invalid? Also, please note that "starting point" may be other than full hour (and possibly expressed in something other than local time). I mean intervals similar to "07:30 + n × PT20M" and use cases like "run this task every P1D12H starting at 2001-01-01T06:00:00Z". Maybe something like
could be more useful? |
Thanks for the feedback @perceptron8
Not saying they are invalid. I only looked at rounding towards a fraction of the hour. Did not take a whole day into consideration.
Agreed. That is nice to have, but isn't this more cron-ish? I think this would go into a different direction than the problem I am trying to solve. Anyways, I did clean up the code and added a bit more documentation. Not saying I am done. Does it look like I am going into the right direction? |
This all gets complicated because there are different ways to look at the problem.
I believe the first two can be handled with a single The method name(s) need to flow when imported. I think three separate methods probably makes this clearest:
The duration would need to be divisible into a day. The implementation would work based off the nano-of-day. |
Hi there,
this PR contains four TemporalAdjuster to round time to the nearest part of an hour. Rounding to the nearest part of an hour is often done in payroll systems, scheduling and appointment making.
The code is repetitive and contains hardcoded numbers. Granted not the most clever approach, but it is straightforward and easy to read and maintain.
I know this project emphasizes well written JavaDoc. I am not a native speaker. I tried.