-
Notifications
You must be signed in to change notification settings - Fork 18
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
Support for Period #3
Comments
Agreed! A contribution here would be great ;) |
No promises, but I'll have a look :) |
Would this just be a matter of copying Duration's implementation and tweaking it a bit? |
TBH - I haven't worked with periods much... and looking into it again I think my hesitation in implementing it was finding the best way to store it in SQL in such a way that it wouldn't lose any information and could also be queried with operators other than We could "Normalize" the value before storing but then we lose the ability to store negative values. For example, a period of 1 day, -1 hours would be normalized to 23 hours. It's also not clear what datatype to pick: a sql We could try storing it as some string, ie "YY:MM:WW:DD:HH:MM" etc (I know those aren't the exact format strings but you get the picture). But doing so makes it difficult to perform any sort of querying of this data since it would probably have to be converted to another type first to be able to use operators like Anyway, lots of things to work out before implementing it :) |
No worries, does indeed sound complicated and until there's enough demand can probably sit in the too hard basket :) Not only negative values, but non-normalized values too, like 90minutes is valid and the equality check is each property being equa, so you'd want to maintain those in/out of the db as is, not matter how bizarre the values :P It actually sounds like NodaTime doesn't support greater than, less than, etc. type "ordering" operators.
If you look at a type like Duration --> click Operators (side menu), you can see it has tonnes more than Period does. So that got me thinking the others aren't supported and that does indeed look to be the case. Sample. So if the compiler won't even allow The trickier part then becomes mappings for |
Ooohhhh.... interesting. I didn't see that It looks like the This format allows us to implement the queries like I need to investigate further to see if it covers all the scenarios (methods & properties) that we'd want. Thoughts? The |
I don't personally have a need for this immediately or can think of any future need at the moment. That's to say:
Using NodaTime's string format definitely makes sense, explicitly using Interesting stuff in any case! |
Wouldn't normalizing destroy the purpose of why it exists in the first place? Check out this. |
@HerrNiklasRaab - agreed. We would most likely use the |
From the NodaTime docs
I understand that as:
Period
should be used with theLocal*
types andDuration
with theZoned*
types.So
Period
support would be nice :)The text was updated successfully, but these errors were encountered: