C
C
campus12017-09-24 16:39:30
C++ / C#
campus1, 2017-09-24 16:39:30

A small code review?

Guys, I'm learning C#. I do different things.
Here is one of them => The interval [a, b] takes one of three values: [40, 50], [100, 90], [-300, 290].
Find the minimum multiple of 5 and the maximum multiple of 5.
This is how I solved it =>

static void Main(string[] args)
        {
            int a = 0;
            int b = 0;
            Console.WriteLine("Enter number");
            int X = Convert.ToInt32(Console.ReadLine()); 
            int[] numbers = new int[X];
            Console.WriteLine("Choose interval:\n" + "1) -40 50;\n" + "2) -100 90;\n" + "3) -300 290;\n");
            int N = Convert.ToInt32(Console.ReadLine());
            switch (N)
            {
                case 1:
                    a = -40; b = 50;
                    break;
                case 2:
                    a = -100; b = 90;
                    break;
                case 3:
                    a = -300; b = 290;
                    break;
                default:
                    Console.Write("Error. You dont choose correct interval ");
                    break;
            }
            Random rnd = new Random();
            int rndNumb;
            for (int i = 0; i < X; i++)
            {
                rndNumb = rnd.Next(a, b);
                numbers[i] = rndNumb;  
            }
            int max = numbers[0];
            int min = numbers[0];
            for (int i = 1; i < X; i++)
            {
                if (numbers[i] > max && i % 5 == 0)
                {
                    max = numbers[i];
                }
                if (numbers[i] < min && i % 5 == 0)
                {
                    min = numbers[i];
                }
            }
            Console.Write("Maximum even element  is : {0}\n", max);
            Console.Write("Minimum even element is : {0}\n\n", min);
        }

Please tell me if the code is written normally or is it still shitty code?
Thank you very much!

Answer the question

In order to leave comments, you need to log in

1 answer(s)
A
Arseniy Efremov, 2017-09-24
@campus1

If you want a code review , then here:
1. Using Convert.ToInt32 instead of Int32.TryParse (or exception handling ) leads to an unexpected program termination in some cases.
2. When you get into the default block, you will simply exit the switch control, after which the program will continue and its work will turn out to be incorrect.
3. The cycle with the Random.Next call can be reduced to 1 line by removing the brackets and the extra variable.
4. It would be possible to break such a large Main into several functions. There is not much practical use in this case, but it would increase readability, and indeed.
5. Are you sure that it is the loop counter in the conditions that needs to be divided by 5 there?

Didn't find what you were looking for?

Ask your question

Ask a Question

731 491 924 answers to any question