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:
- Number of rows
- Number of columns
- 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.
