-2

In a code like this

public static void Sub()
    {
        Console.WriteLine("Ile liczb chcesz odjąć?");
        RetryPoint1:
        try
        {
            int liczby = Convert.ToInt32(Console.ReadLine());
            int[] cyfry = new int[liczby];
            Console.WriteLine("Będziesz odejmować " + liczby + " liczb");
            for (int i = 0; i < liczby; i++)
            {   
                RetryPoint2:
                try
                {
                    Console.WriteLine("Wpisz " + (i + 1) + " liczbę");
                    cyfry[i] = Convert.ToInt32(Console.ReadLine());
                }
                catch
                {
                    Console.WriteLine("Nie liczba!");
                    goto RetryPoint2;
                }  
            }                   
            int wynik = cyfry[0];
            for (int i = 1; i < liczby; i++)
            {
                wynik = wynik - cyfry[i];
            }
            Console.WriteLine("Wynik to " + wynik);
        }
        catch
        {
            Console.WriteLine("Nie liczba!");
            goto RetryPoint1;
        }
    }

there are two points where same exception(not int as user input) can occur. Is there some way to use retry point as variable that changes depending on when this error happened? Is it even a "valid" way of doing things in this case? Once can imagine that those try catches could start nesting quickly if there were more points within one function where the same exception could happen. I know I could put next try catch below for better readeability, but if there's same exception occurting every time, isn't there some more "right" way to handle it, with just one try catch?

anonim
  • 9
  • 3
  • 5
    Using of `goto` [is not recommended](https://stackoverflow.com/questions/46586/goto-still-considered-harmful) – Fabjan Feb 07 '19 at 17:23
  • 2
    My personal opinion, I'd never use goto. – Trey Feb 07 '19 at 17:23
  • 3
    It appears that you're relying on exception handling to deal with invalid input. Using `int.TryParse` will eliminate the need for exception handling and the `goto`s. Then you can use small loops for retrying on invalid input. – madreflection Feb 07 '19 at 17:25
  • 3
    You're catching all exception types. Are you sure you would want to catch, for example, OutOfMemoryException and handle it this way? It might make more sense to use int.TryParse instead of using exceptions for control flow. And the gotos can be rewritten with while loops. – Wim Coenen Feb 07 '19 at 17:26
  • 1
    Say a Big NO to Goto.. – Soumen Mukherjee Feb 07 '19 at 17:28
  • Is it just me or you just changed the whole code? – Daniel Reyhanian Feb 07 '19 at 17:33

1 Answers1

0

You shouldn't use goto at all for "RetryPoints" here. It's strongly advised to not use goto in scenarios like this.

Also don't use Convert.ToInt32 to handle user input, use int.TryParse instead.

This is how your code could look like without using goto and using int.TryParse in favor of Convert.ToInt32:

public static void Sub()
{
    Console.WriteLine("Ile liczb chcesz odjąć?");

    int liczby;
    while (!int.TryParse(Console.ReadLine(), out liczby))
        Console.WriteLine("Nie liczba!");

    int[] cyfry = new int[liczby];

    Console.WriteLine("Będziesz odejmować " + liczby + " liczb");

    for (int i = 0; i < liczby; i++)
    {
        while (true)
        {
            Console.WriteLine("Wpisz " + (i + 1) + " liczbę");

            if(int.TryParse(Console.ReadLine(), out cyfry[i])) break;

            Console.WriteLine("Nie liczba!");
        }
    }

    int wynik = cyfry[0];

    for (int i = 1; i < liczby; i++)
        wynik -= cyfry[i];

    Console.WriteLine("Wynik to " + wynik);
}
Tobias Tengler
  • 6,848
  • 4
  • 20
  • 34