-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Make ToFileTime understand DateTime.MinValue #6288
Comments
this is not really true. DateTime.MinValue is known date and cannot be used as unknown value especially if you are going to use it in subsequent operations like ToFileTime for instance. looks to me you need to use Nullable type and not DateTime.MinValue |
It is true that |
@magol default(somthing) doesn't mean this is uninitialized. it means it is initialized with the default value. when you do something like
for reference types too, string s = default(string); is equivalent to string s = null; // this is still initialization even we assign it to null. this is why I recommended to use Nullable to differentiate between the initialized and uninitialized DateTiem. I am closing the issue but please reply back or reopen it if you have any more feedback. Thanks. |
DateTime.ToFileTime
When you want to denote an unknown date in .NET, you normally use
DateTime.MinValue
ordefault(DateTime)
.The equality for a FileTime is
0
(See WIN32_FILE_ATTRIBUTE_DATA structure)But, if you try to use
DateTime.ToFileTime(DateTime.MinValue)
you get anArgumentOutOfRangeException
.My suggestion is to make
DateTime.ToFileTime
to returns0
if it getsDateTime.MinValue
as input.DateTime.FromFileTimes
Even in the opposite situation, when you want to use
DateTime.FromFileTime
, have an issue as I see it. If the FileTime is0
, it returnsnew DateTime(1601, 1, 1, 1, 0, 0)
. It is not entirely wrong, and is manageable. But I think it is semantically wrong when it returns something that looks like a specific date. I suggest that it returnsDateTime.MinValue
if it gets0
as input.That is an braking change, but I see it as acceptable when I do not think that this special case is not so common
The text was updated successfully, but these errors were encountered: