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

Comments

Projects
None yet
2 participants
@GoogleCodeExporter

GoogleCodeExporter commented Mar 15, 2015

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

This comment has been minimized.

Show comment
Hide comment
@GoogleCodeExporter

GoogleCodeExporter Mar 15, 2015

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 commented Mar 15, 2015

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

This comment has been minimized.

Show comment
Hide comment
@GoogleCodeExporter

GoogleCodeExporter Mar 15, 2015

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 commented Mar 15, 2015

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

This comment has been minimized.

Show comment
Hide comment
@GoogleCodeExporter

GoogleCodeExporter Mar 15, 2015

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 commented Mar 15, 2015

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

This comment has been minimized.

Show comment
Hide comment
@GoogleCodeExporter

GoogleCodeExporter Mar 15, 2015

Fixed in revision ff469eb394c2.

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

  • Changed state: Fixed

GoogleCodeExporter commented Mar 15, 2015

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