CODEWITHSUNDEEP
c#datetimeresharperdefault-value
B. Clay Shannon-B. Crow Raven
B. Clay Shannon-B. Crow Raven

Reputation: 10247

Would I really want to return the minimum date?

An old work colleague used to quote his father about tools, "You have to be smarter than it."

In the code below, Resharper is telling me, "Value assigned is not used in any execution path" (pointing to the first line). If I accept its offer of help, dt is not assigned a value ("today").

Is this a case where "I have to be smarter than it" and ignore their warning, or is this a case where the tool is smarter than me, and I'm just not understanding it?

My take on the situation is that if the if statement fails, the current date is returned (the default value I want), but if I acquiesce to Resharper's "demands" it would return the default value for Datetime, which is the minimum date, which I assume is something like 7/4/1776 or 1/1/0000 or so.

DateTime dt = DateTime.Now;
if (!(DateTime.TryParse(substr, out dt))) {
    using (var dtpDlgForm = new ReturnDate("Please select the Date that the file was created:")) {
        if (dtpDlgForm.ShowDialog() == DialogResult.OK) {
            dt = dtpDlgForm.ReturnVal;
        } 
    }
}
return dt;

Upvotes: 21

Views: 1876

Answers (7)

Jon Senchyna
Jon Senchyna

Reputation: 8047

It's because you're assigning a value to dt and then passing it in as an out param. If a variable is passed in as an out param:

  • It must be assigned a value inside that function before being used within it
  • It must be assigned a value before that function returns

In your case, you can fix the message from ReSharper message by changing your first line to:

DateTime dt;

My take on the situation is that if the if statement fails, the current date is returned

This is not true. It will always return DateTime.MinValue, regardless of the value of the DateTime object you pass in.

From MSDN - DateTime.TryParse(string, out DateTime):

result

Type: System.DateTime%

When this method returns, [result] contains the DateTime value equivalent to the date and time contained in s, if the conversion succeeded, or MinValue if the conversion failed. The conversion fails if the s parameter is null, is an empty string (""), or does not contain a valid string representation of a date and time. This parameter is passed uninitialized.

(Emphasis added)

Upvotes: 20

Douglas
Douglas

Reputation: 54887

Your intended logic allows for three possible return values of DateTime (in order of preference):

  1. The parsed value of substr.
  2. The value selected from the dialog.
  3. The current date and time.

You can implement this logic by having separate return statements that are executed when their condition succeeds:

DateTime dt;
if (DateTime.TryParse(substr, out dt))
    return dt;

using (var dtpDlgForm = new ReturnDate(
    "Please select the Date that the file was created:")) 
{
    if (dtpDlgForm.ShowDialog() == DialogResult.OK)
        return dtpDlgForm.ReturnVal;
}

return DateTime.Now;

Edit: For an explanation on why you should not assign a value to a variable that will be used as an out parameter, refer to Mark Byers’s answer.

Upvotes: 23

archil
archil

Reputation: 39501

The point here is about using out parameter modifier:

Although variables passed as out arguments do not have to be initialized before being passed, the called method is required to assign a value before the method returns.

DateTime.TryParse will assign a default value to dt anyways, and if it cannot parse substr, the resulting dt will have the value of the minimum datetime.

Upvotes: 7

Mark Byers
Mark Byers

Reputation: 838266

The answer you accepted shows what you should be doing but doesn't explain why Resharper complains in the first place. Since this explanation could be useful for others who find your question, here it is:

You should follow Resharper's advice and change the first line to:

DateTime dt;

This declares the variable dt but does not assign any value to it. There is no need to assign a value here because it will be definitely assigned on the next line due to the out keyword. From the documentation:

Although variables passed as out arguments do not have to be initialized before being passed, the called method is required to assign a value before the method returns.

Emphasis mine. Assigning the value DateTime.Now is unnecessary and misleading because this value will never be used.

My take on the situation is that if the if statement fails, the current date is returned

That is not what your code does. From the documentation:

result: When this method returns, contains the DateTime value equivalent to the date and time contained in s, if the conversion succeeded, or MinValue if the conversion failed.

With the code you posted if the parse fails then dt will contain the value DateTime.MinValue and not the value DateTime.Now that you assigned.

Upvotes: 53

Hans Z
Hans Z

Reputation: 4744

DateTime.TryParse(substr, out dt);

can return false but it will still modify dt. It will try to complete dt to the best of its ability, initializing some of the values as best as it can. When using the out modifier in c#, you are telling the program to initialize it, and you should not expect to retain the value you pass in.

What you could do is

DateTime dt;
if (!(DateTime.TryParse(substr, out dt))) {
    using (var dtpDlgForm = new ReturnDate("Please select the Date that the file was created:")) {
        if (dtpDlgForm.ShowDialog() == DialogResult.OK) {
            dt = dtpDlgForm.ReturnVal;
        } 
        else {
            dt = DateTime.Now;
        }
    }
}

return dt;

Upvotes: 3

comecme
comecme

Reputation: 6386

This code would make the warning go away. But I think Douglas' answer is easier to read.

DateTime dt;
if (!(DateTime.TryParse(substr, out dt))) {
    dt = DateTime.Now;
    using (var dtpDlgForm = new ReturnDate("Please select the Date that the file was created:")) {
        if (dtpDlgForm.ShowDialog() == DialogResult.OK) {
            dt = dtpDlgForm.ReturnVal;
        } 
    }
}
return dt;

Upvotes: 3

Olivier Jacot-Descombes
Olivier Jacot-Descombes

Reputation: 112372

An out parameter has always a value assigned to it. It is always guaranteed that the called function assigns a value to it before returning. Therefore it will overwrite the value initially assigned in any case.

Upvotes: 8

Related Questions