Ytukyg

I've been trying to make a matrix console application in C# but I am struggling when it comes to checking viability of user input. Here's the code:

//INPUT OF VALUES INTO MATRIX A BY ROWS

int ValuesCounter = 2; //index for new arrays created on fail.

string valuesValuesCounter = Console.ReadLine().Split(' '); //read user input into string array

int valuescheckValuesCounter = new int[valuesValuesCounter.Length]; //test array to check if user input can be parsed into int

for (int i = 0; i <= valuesValuesCounter.Length; i++) // loop for checking parseability of individual array items

{

    while(!int.TryParse(valuesValuesCounter[i], out valuescheckValuesCounter[i])) //same

    {

        /*if it can't be parsed, create new array and try again*/

        ValuesCounter += 2;

        Console.WriteLine("Wrong values! Please try again!");

        valuesValuesCounter = Console.ReadLine().Split(' ');

        valuescheckValuesCounter = new int[valuesValuesCounter.Length];

    }

}

I doubt this is very efficient. I've tried using ArrayList instead because you can just remove the values in it, but it can't be split so I don't know how to make it work. What is a better way to do this?

Also tried conducting my own research, but didn't find any solution (maybe it's there and I am just stupid for not seeing it, I don't know).

How to delete an array in c#?
Convert 2D string array into 2D int array (Multidimensional Arrays)
C# arraylist user input
int.TryParse out into an object array (this seems pretty close but I don't see anything there to retry the input in case it can't be parsed).

So there you have it. I'm pretty dumb so if this question seems stupid it's because the one asking it is stupid.

First of all, make the user experience as good as possible. Inputing a matrix all at once can be error prone, make the user's life easier and take it step by step.

Second, make small methods that perform simple tasks; they are easier to reason about, easier to write, easier to debug and easier to modify if new requirements come along. Don't make methods do too much, it will only buy you headaches and more debugging hours than necessary. If you are learning, don't worry if it seems you are breaking down to seemingly absurdly simple tasks, you wont regret it. Always remember that the most complicated problems you can imagine are always solved solving smaller and easier ones.

So, the first thing you need is a method that prompts the user for a valid integer, so lets try that:

private static int GetIntegerFromUser(string prompt)

{

    int value;

    Console.Write($"{prompt}: ");



    while (!int.TryParse(Console.ReadLine(), out value))

    {

        Console.WriteLine("That is not a valid value. Please try again.");

        Console.Write($"{prompt}: ");

    }



    return value;

}

See how that works? This method has one single goal in life; get the user to enter a valid integer. Never mind what the integer is for, it doesn't care, thats not its job, this method will keep asking the user until its goal in life is achieved.

Ok, whats next? When filling in a matrix we'll need to know:

1. Number of rows


2. Number of columns


3. Values

Ok lets write some methods that fetch just that:

private static (int Rows, int Columns) GetMatrixSize()

{

    var rows = GetIntegerFromUser("Please enter number of rows");

    var columns = GetIntegerFromUser("Please enter number of columns");

    return (rows, columns);

}

And,

private static int[,] GetMatrixValues(int rows, int columns)

{

    var matrix = new int[rows, columns];



    for (var row = 0; row < rows; row++)

    {

        for (var column = 0; column < columns; column++)

        {

            matrix[row, column] =

               GetIntegerFromUser($"Enter matrix value [{row}, {column}]");

        }

    }

}

Now you should come to realize why making small methods that do simple tasks is such a great idea. Because chances are really good you'll get to reuse them and once you know the method works correctly, it will work correctly everywhere.

Ok, we've got all we need, now its just a matter of putting it all together:

public static int[,] GetMatrixFromUser()

{

    var size = GetMatrixSize();

    return GetMatrixValues(size.Rows, size.Columns);

}

How easy does that read?

So, we're done! Really? Well... no. We have no value validation at all, and that can be a problem. What happens if someone decides to input a negative number of rows? Ok, no problem, lets make a method that makes sure a value is inside a valid range:

private static bool IsInRange(int value, 

                              int lowerInclusiveBound,

                              int upperExclusiveBound,

                              string messageOnFailedValidation)

{

    if (value >= lowerInclusiveBound &&

        value < upperExclusiveBound)

        return true;



    Console.WriteLine(messageOnFailedValidation);

    return false;

}

And now, becuase we've used small methods that do well defined tasks, adding the new requirement is easily done modifying our GetMatrixSize method a little:

private static (int Rows, int Columns) GetMatrixSize()

{

    int rows, columns;



    do

    {

        rows = GetIntegerFromUser("Please enter number of rows");

        columns = GetIntegerFromUser("Please enter number of columns");

    } while (!IsInRange(rows,

                        1,

                        int.MaxValue,

                        "Number of rows must be equal or greater than one.") |

             !IsInRange(columns,

                        1,

                        int.MaxValue,

                        "Number of columns must be equal or greater than one."));



    return (rows, columns);

}

Yeah, now we are done...

Almost! One last thing. Another good habit is making sure the truths you believe in are actually true. What are you believing to be true inside GetMatrixValues? You believe that rows and columns will have valid values. But your belief is purely based on faith, you are not checking if this is true.

But wait... I already checked in GetMatrixSize right? Yes, but what guarantee do you have that GetMatrixValues won't be called from some other method later on? Reusability remember?

Here you can go two paths. You can throw an exception if the values are not valid or you can assert that the values are in fact valid. Because the method is private and having invalid values probably means a bug somewhere up the callstack, the better option here is to assert your truths:

private static int[,] GetMatrixValues(int rows, int columns)

{

    Debug.Assert(rows > 0 && columns > 0);

    var matrix = ...

}

Any other truths merely on faith? Who is making sure that upperExclusiveBound and lowerInclusiveBound in IsInRange actually make sense? Is someone stopping you from making the following call: IsInRange(10, 100, -100, ...)? A call like that is probably a bug somewhere else in your code, make your life easier by making these kind of bugs easier to spot. Again, assert your truths:

private static bool IsInRange(int value, 

                              int lowerInclusiveBound,

                              int upperExclusiveBound,

                              string messageOnFailedValidation)

{  

    Debug.Assert(upperExclusiveBound > lowerInclusiveBound);



    if ...

}  

A good case could also be made on wether you should be checking that prompt in GetIntegerFromUser and messageOnFailedValidation in IsInRange are not empty or null strings, etc.

Yeah, now we are really done.

You do not need the while loop inside the for loop. just check each item for validity and break, if you encounter an invalid string, of course if all were valid (loop will go to the end) and i will be equal to length of your array:

bool success = false;

do

{

     int i = 0;

     string valuesValuesCounter = Console.ReadLine().Split(' ');

     int valuescheckValuesCounter = new int[valuesValuesCounter.Length];

     for (int i = 0; i <= valuesValuesCounter.Length; i++)

        if(!int.TryParse(valuesValuesCounter[i], out valuescheckValuesCounter[i]) break;

     success = i == valuesValuesCounter.Length;

}while(!success);

Another way would be using Linq:

do

{

    int i = 0;

    string valuesValuesCounter = Console.ReadLine().Split(' ');

    int valuescheckValuesCounter = new int[valuesValuesCounter.Length];

    success = valuesValuesCounter.All(x => int.TryParse(x, out valuescheckValuesCounter[i++]);

}while(!success);