3

I've been learning C# for a little over a month. I am working on an exercise where I ask the user to enter a time in a 24-hour clock format and check if it's valid.

That's not important though. My problem is I'm confused about an error. The below code creates an unhandled exception, and says my input string was not in a correct format. It specifies line 22. (The hour variable.)

Now, I've already fixed it, by moving all the variables except userInput inside the try block. But I'm confused why that fixed it. I'm very new, and have tried googling, but honestly don't really know how to even phrase my question.

The complete (pre-fixed) code is below. I appreciate everyone's patience.

{
    class Program
    {
        static void Main(string[] args)
        {
            Console.WriteLine("Please enter a time value in the 24-hour time format. (ex. 19:00)");
            var userInput = Console.ReadLine();
            var userComponents = userInput.Split(':');
            var hour = Convert.ToInt32(userComponents[0]);
            var minute = Convert.ToInt32(userComponents[1]);    

            if (String.IsNullOrWhiteSpace(userInput))
            {
                Console.WriteLine("Invalid Time");
                return;
            }

            try
            {                      
                if (hour <= 23 && hour >= 00 && minute >= 0 && minute <= 59)
                    Console.WriteLine("Ok");
                else
                    Console.WriteLine("Invalid Time");
            }

            catch(Exception)
            {
                Console.WriteLine("Invalid Time");
            }
       }
    }
}

Someone requested I post the fixed code:

{
    class Program
    {
        static void Main(string[] args)
        {
            Console.WriteLine("Please enter a time value in the 24-hour time format. (ex. 19:00)");
            var userInput = Console.ReadLine();  

            if (String.IsNullOrWhiteSpace(userInput))
            {
                Console.WriteLine("Invalid Time");
                return;
            }

            try
            {
                var userComponents = userInput.Split(':');
                var hour = Convert.ToInt32(userComponents[0]);
                var minute = Convert.ToInt32(userComponents[1]);

                if (hour <= 23 && hour >= 00 && minute >= 0 && minute <= 59)
                    Console.WriteLine("Ok");
                else
                    Console.WriteLine("Invalid Time");
            }

            catch(Exception)
            {
                Console.WriteLine("Invalid Time");
            }
       }
    }
}

Someone also requested the debugger info:

System.IndexOutOfRangeException HResult=0x80131508 Message=Index was outside the bounds of the array. Source=Section 6 24 Hour Time
StackTrace: at Section_6_24_Hour_Time.Program.Main(String[] args) in D:\Repos\Mosh C# Udemy\Exercises\C# Fundamental Exercises\Section 6 24 Hour Time\Section 6 24 Hour Time\Program.cs:line 23

  • What input did you give it to make it crash? – Sweeper Nov 04 '18 at 10:48
  • Can you share your post-fixed code too, just to make it more clear? It shouldn't have fixed anything, it just should have handled an exception. The cause of this exception is usually your string being not a valid number - try to debug it and see what the values inside `userComponents` are. Update your question if it seems to be valid. – Yeldar Kurmangaliyev Nov 04 '18 at 10:51
  • I gave several inputs to make it crash. Ranging from a simply typing nothing and hitting enter, to just typing something like 00. I added the post-fixed code. I also ran the debugger. I'm embarrassed to admit, but I actually wasn't really aware of that. I'm adding what the debugger said as well. – Caleb Twombly Nov 04 '18 at 10:57
  • The only way for the original code to have that specific error is if you have something not numeric before the colon, i.e. if you typed an alpha character in there, or had nothing before the colon. Use this as a good lesson in why you should test user input before trusting it. – slugster Nov 04 '18 at 11:01
  • 1
    You're splitting, and then always use index `0` and index `1` which don't need to be filled at all causing an index out of range exception when trying to access index `0` or `1`. Try to check if split actually gives the results you want it to give – Baklap4 Nov 04 '18 at 11:04
  • Yeah, I realize now that trying to convert to an int before checking to make sure there is a string to convert was causing problems. Live and learn! – Caleb Twombly Nov 04 '18 at 11:05
  • Possible duplicate of [What is an IndexOutOfRangeException / ArgumentOutOfRangeException and how do I fix it?](https://stackoverflow.com/questions/20940979/what-is-an-indexoutofrangeexception-argumentoutofrangeexception-and-how-do-i-f) – Caramiriel Nov 04 '18 at 11:13

4 Answers4

3

As said in the comments, you're running str.split() and then just accessing the output of it with index 0 and index 1. Upon requesting index 0 or 1 whilst it's not there you'll get an Index out of range exception telling you Item on index 0 or 1 is not there.

Then there's the issue with Convert.ToInt32 as you don't catch an overflowexception or formatexception.

class Program
{
    static void Main(string[] args)
    {
        Console.WriteLine("Please enter a time value in the 24-hour time format. (ex. 19:00)");
        var userInput = Console.ReadLine();
        if(string.IsNullOrEmpty(userInput)) {
            Console.WriteLine("No input");
            return;
        }

        if(!userInput.Contains(':')) {
            Console.WriteLine("Input does not have `:` in it. Invalid Time.");
            return;
        }

        var userComponents = userInput.Split(':');
        if(userComponents.Length != 2) { 
            Console.WriteLine("Invalid Time");
            return;
        }
        if(string.IsNullOrEmpty(userComponents[0]) || string.IsNullOrEmpty(userComponents[1]) {
            Console.WriteLine("No hours or minutes given. Invalid Time");
            return;
        }
        try {
            var hour = Convert.ToInt32(userComponents[0]);
            var minute = Convert.ToInt32(userComponents[1]);    
        } catch(OverFlowException e) {
            // Do something with this.
            return;
        } catch (FormatException e) {
            // Do something with this.
            return;
        }

        if (hour <= 23 && hour >= 00 && minute >= 0 && minute <= 59)
            Console.WriteLine("Ok");
        else
            Console.WriteLine("Invalid Time");

   }
}

Edit

As mentioned by @ckuri and What's the main difference between int.Parse() and Convert.ToInt32 one should prefer int.TryParse() over Convert.ToInt32 since we're dealing with user input here.

  • If you've got a string, and you expect it to always be an integer (say, if some web service is handing you an integer in string format), you'd use Int32.Parse().

  • If you're collecting input from a user, you'd generally use Int32.TryParse(), since it allows you more fine-grained control over the situation when the user enters invalid input.

  • Convert.ToInt32() takes an object as its argument. (See Chris S's answer for how it works)

Convert.ToInt32() also does not throw ArgumentNullException when its argument is null the way Int32.Parse() does. That also means that Convert.ToInt32() is probably a wee bit slower than Int32.Parse(), though in practice, unless you're doing a very large number of iterations in a loop, you'll never notice it.

Baklap4
  • 3,914
  • 2
  • 29
  • 56
  • Added code example based upon the `pre-fix` solution. – Baklap4 Nov 04 '18 at 11:22
  • 1
    When handling user input instead of Convert.ToInt32 one should use int.TryParse which also avoids raising exceptions and thus provides a much more cleaner and more performant code. – ckuri Nov 04 '18 at 11:45
  • @ckuri since the OP started this question with `Convert.ToInt32()` i answered it as much as possible by still using `Convert.ToInt32()`. Edited the answer with the differences between those methods and added a link to why we should use int.TryParse. – Baklap4 Nov 08 '18 at 21:33
2

Before you access the elements of the array, check if it has the desired length:

if (userComponents.Length < 2)
{
    Console.WriteLine("Invalid Time");
    return;
}

var hour = Convert.ToInt32(userComponents[0]);
var minute = Convert.ToInt32(userComponents[1]);

Note that the input string may not contain the colon, so Split will return an array with only one element. In such case userComponents[1] does not exist, hence the exception.

BartoszKP
  • 34,786
  • 15
  • 102
  • 130
0

This error indicates that you are trying to convert a string value to int (in this case) that is not castable. So you should make sure that the value you input after splitting is castable to int, also it is nice to Trim() them before casting:

 var hour = Convert.ToInt32(userComponents[0].Trim());
 var minute = Convert.ToInt32(userComponents[1].Trim()); 
Ashkan Mobayen Khiabani
  • 33,575
  • 33
  • 102
  • 171
-1

before you try to split userInput you should first check for IsNullOrWhiteSpace

  • Oh, I think I get it, if I try to convert to an integer before making sure the input is correct the program stalls out when it tries to convert. It makes sense when I think about it. – Caleb Twombly Nov 04 '18 at 11:03
  • Please, update your answer with more details on how it should be checked and why. Now this answer is of poor quality. – Yeldar Kurmangaliyev Nov 04 '18 at 11:04