I have a method that (sometimes) takes in a string in the format "dddd MMMM dd"
(Monday January 04) that needs to get parsed into a DateTime. I say sometimes because it may also get passed in "Today"
or "Tomorrow"
as the value.
The code to handle this was simple enough:
if (string.Compare(date, "Today", true) == 0)
_selectedDate = DateTime.Today;
else if (string.Compare(date, "Tomorrow", true) == 0)
_selectedDate = DateTime.Today.AddDays(1);
else
_selectedDate = DateTime.Parse(date);
This worked until halfway through December. Some of you have probably already spotted what went wrong.
This would have failed on any date in the New Year with the error:
"String was not recognized as a valid DateTime because the day of week was incorrect."
It was getting passed "Monday January 04"
which is a valid date for 2010, but not in 2009.
So my question is: Is there any way to set the year either for the current year or the next year? Right now, as a quick and dirty fix, I have this:
if (!DateTime.TryParseExact(date, "dddd MMMM dd", CultureInfo.InvariantCulture, DateTimeStyles.None, out _selectedDate))
if (!DateTime.TryParseExact(date + " " + (DateTime.Now.Year + 1), "dddd MMMM dd yyyy", CultureInfo.InvariantCulture, DateTimeStyles.None, out _selectedDate))
throw new FormatException("That date is not valid.");
So it will try to parse it using the current year, and if it is unsuccessful it will try again using the next year. If it fails after that, it'll just assume it's an invalid date because I only need to worry about 1 year in advance, but if anyone has a more flexible solution, I'd appreciate it. (Note, I don't need to worry about validating the date that gets passed in, it will be valid for either the current or following year).
First, your unit testing should have caught this. You might want to revisit the tests that you wrote for this method to learn from this experience on how to cover your functionality more throughly.
Second, is there any particular reason why you are using
String.Compare
instead ofString.Equals
? I consider the following more readable:I think it more clearly reads what is happening (especially as we don't have to remember what the final
bool
parameter means inString.Compare
).Now, to get the heart of your question. Your method is perfectly fine and very clearly expresses the logic. I would make one small refactoring however:
This separates your method into two distinct pieces of functionality and prevents from repeating yourself (
CultureInfo.InvariantCulture
andDateTimeStyles.None
) making testing and maintenance a little easier. (You probably want a better method name thanParse
; I chose a short one to prevent the scroll bar from appearing in the code window here.)As one last caveat (without knowing the details of your system) you might want to consider also checking the prior year! Just imagine the following situation:
Just something to consider depending on the nature of your system.