6
\$\begingroup\$

I'm working on a project where I have to prompt a user a enter several dates. This is the code I came up with for prompting the user for the day, month, and year.

Console.WriteLine("Day: ");
var dateDay = Console.ReadLine();
var dateDayInt = Convert.ToInt32(dateDay);

Console.WriteLine("Month: ");
var dateMonth = Console.ReadLine();
var dateMonthInt = Convert.ToInt32(dateMonth);

Console.WriteLine("Year: ");
var dateYear = Console.ReadLine();
var dateyearInt = Convert.ToInt32(dateYear);

DateTime myDate = new DateTime(dateYearInt, dateMonthInt, dateDayInt, 00, 00, 00, 000);

I'm using this date in a method, so it needs to be in this format.

As you can see, it's a ton of repetitive code. If I even add one or two more dates, my project will become very cluttered. Let's say I need to get 20 dates. Is there an easier way to do this, like only having one prompt per date?

\$\endgroup\$
  • 6
    \$\begingroup\$ DateTime has a pretty powerful string parser built in. It can handle many different formats. Rather than explicitly getting each part of the date, you might try getting the entire date string in whatever format the user feels like entering and passing it to DateTime.TryParse(). See msdn.microsoft.com/en-us/library/ch92fbc1(v=vs.110).aspx \$\endgroup\$ – Zack Feb 18 '16 at 23:01
8
\$\begingroup\$

Note that I'm not very familiar with C#, so the code that I write may not actually compile.


Let's say I need to get 20 dates. Is there an easier way to do this, like only having one prompt per date?

There's a much easier way to do this, and that is to extract this to it's own separate method. This method should do just what your code is doing now: prompt the user for the day, the month, and the year, and should return a DateTime.

public static DateTime PromptDateTime()
{
    Console.WriteLine("Day: ");
    var day = Convert.ToInt32(Console.ReadLine());

    Console.WriteLine("Month: ");
    var month = Convert.ToInt32(Console.ReadLine());

    ... year...

    return new DateTime(year, month, day, 0, 0, 0, 0);
}

Need to call this method twenty times? Go ahead (you'd probably want to use a loop though). With an extracted method like the one above, you won't have to worry about having cluttered code.


Validate Input

This is how you are getting input:

var dateDay = Console.ReadLine();
var dateDayInt = Convert.ToInt32(dateDay);

What if the user enters something like this?

LOOK MA! NO VALIDATION

What's your code going to do? It's going to throw an error because Convert.ToInt32 doesn't quite know how to parse that.

To validate the user's input, you should use int.TryParse. This method takes a string and an output variable and attempts to parse the string into a number and puts the parsed number in the output variable. If the parse is unsuccessful, then this method returns false.

What this method could look like in action:

var input = Console.ReadLine();
var num;
if(int.TryParse(input, out num))
{
    Valid!
} else
{
   Invalid!
}

Of course, this is a little much to put inside this method you have (considering the fact that you'll have to keep looping to get the user's proper input). Therefore, you should extract this into a method.

public static int ReadInteger() {
    while(true) {
        var input = Console.ReadLine();
        var num;
        if(int.TryParse(input, out num))
        {
            return num;
        }
    }
}

The above method keeps on looping until proper input is receive. As you may notice, this is not very suitable for unit testing as STDIN is forced. However, as Dan Lyons commented:

Console.WriteLine utilizes Console.Out for everything - if the method took in a TextWriter, he could use Console.Out in "production", but a StringWriter in test.

Now, with proper validation, your extracted method would look like this:

public static DateTime PromptDateTime()
{
    Console.WriteLine("Day: ");
    var day = ReadInteger();

    Console.WriteLine("Month: ");
    var month = ReadInteger();

    ... year...

    return new DateTime(year, month, day, 0, 0, 0, 0);
}

As svick mentioned, this is still technically not enough validation. What if, for the month, the user enters

13

That doesn't make much sense, does it?

This, of course, can lead to some complications. Some months had 30 days and others have 31. Validation in general should be mostly simple; you just check that the month number is between 1 and 12 (or 0 and 11, but be consistent), and that the year number is between 1 and the current year (unless you want to handle BC/BCE, in which you'd have to do some extra checking).

However, this still doesn't solve the days-in-month problem. A solution could be to create a map/table showing the months and the corresponding amount of days and use that to look up whether the day number is valid. However, then you encounter February (yay!) with which you have to do further validation. Honestly, your best bet would be to go with Zack's solution as it is more idiomatic and would probably be easier to do parsing with (although, I don't know how it handles that days-in-month issue either).

\$\endgroup\$
  • 1
    \$\begingroup\$ Don't try to inject Console, it's pointless. \$\endgroup\$ – Jeroen Vannevel Feb 18 '16 at 22:56
  • \$\begingroup\$ @JeroenVannevel But wouldn't it allow for easier unit testing? Or, am I missing something? \$\endgroup\$ – SirPython Feb 18 '16 at 22:57
  • 3
    \$\begingroup\$ It's a static class: literally pointless. You can't pass it in (and you can't define it as part of the method interface). The only way to extract Console for unit testing is by wrapping it in another class that does allow these things. \$\endgroup\$ – Jeroen Vannevel Feb 18 '16 at 22:59
  • \$\begingroup\$ @JeroenVannevel Good point. I expected it to be an extension of some sort of stream class that would allow for that; guess I didn't read that part in the documentation. Thanks for the fix! I've edited my answer. \$\endgroup\$ – SirPython Feb 18 '16 at 23:03
  • 2
    \$\begingroup\$ @JeroenVannevel Console.WriteLine utilizes Console.Out for everything - if the method took in a TextWriter, he could use Console.Out in "production", but a StringWriter in test. \$\endgroup\$ – Dan Lyons Feb 22 '16 at 19:20
7
\$\begingroup\$

As I mentioned in my comment, DateTime has a built in parser than can handle nearly any date format. See the documentation for a few examples of formats it can handle and how to use DateTime.TryParse().

Additionally, if you need to get multiple dates, you should wrap the logic for getting a date in a method. I suggest something along the lines of:

static DateTime getDateFromUser(string promptText)
{
    DateTime date = DateTime.MinValue;
    do
    {
        Console.WriteLine(promptText);
        string strDateString = Console.ReadLine();
        if(!DateTime.TryParse(strDateString, date))
        {
            Console.WriteLine("That is an invalid date format.  Please try something like '3/14/2015' or '5-22-2015'");
        }
    } while(date == DateTime.MinValue)
    return date;
}
\$\endgroup\$
  • \$\begingroup\$ I don't like how you're using DateTime.MinValue as a sentinel. Instead I would probably use while (true) look with an explicit break, or maybe Nullable<DateTime>. \$\endgroup\$ – svick Feb 22 '16 at 19:58
  • \$\begingroup\$ As you like. It will come out the same either way. \$\endgroup\$ – Zack Feb 22 '16 at 20:00

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.