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

LocalDate.LocalDateTime vs LocalDate.Midnight - naming suggestion #56

Closed
GoogleCodeExporter opened this issue Mar 15, 2015 · 4 comments
Closed
Labels

Comments

@GoogleCodeExporter
Copy link

Not really a feature request, just wonder if this property would be better 
named 'Midnight' since (i) that's what it actually returns, as explained by the 
comment on it and (ii) LocalTime has a corresponding static property called 
'Midnight' that gets midnight time and (iii) In the future you might want to 
add a matching '.Noon' property.


        /// <summary>
        /// Gets a <see cref="LocalDateTime" /> at midnight on the date represented by this local date, in the same calendar system.
        /// </summary>
        public LocalDateTime LocalDateTime { get { return localTime; } }

Original issue reported on code.google.com by HighTech...@gmail.com on 18 Apr 2012 at 6:18

@GoogleCodeExporter
Copy link
Author

I think I'd rather change it from a property to a method at the same time - 
"Midnight" isn't logically a property of a date. The same argument pretty much 
holds for LocalDateTime as a property name, which is another reason to change 
it.

I think if you want to represent "midnight on a date" and "noon on a date" then 
it would be more sensible to have a Noon constant on LocalTime, so you could 
use:

    var noon = date + LocalTime.Noon;
    var midnight = date + LocalTime.Midnight;

(The addition operator is already present, unless my memory's going.)

Will consider what the method should be called...

Original comment by jonathan.skeet on 18 Apr 2012 at 6:41

@GoogleCodeExporter
Copy link
Author

Yes, I quickly stopped using .LocalDateTime and use "date + LocalTime.Midnight" 
instead because the intent is clearer and I'm calculating lots of other 
relevant time ranges like afternoon, evening, night, ... so the pattern is the 
same if I use "+ LocalTime".

Original comment by HighTech...@gmail.com on 18 Apr 2012 at 6:59

@GoogleCodeExporter
Copy link
Author

Right. I think date.AtMidnight() is probably reasonable as a method, as it has 
better discoverability than the + operator (which I'm not planning on removing, 
of course).

Will make the change when I get a chance, which may or may not be tonight...

Original comment by jonsk...@google.com on 18 Apr 2012 at 7:52

@GoogleCodeExporter
Copy link
Author

Fixed in revision ff469eb394c2.

Original comment by jonathan.skeet on 18 Apr 2012 at 10:11

  • Changed state: Fixed

@malcolmr malcolmr added the bug label Mar 15, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants