Project Euler #11 Largest product in a grid

Question

Link to the exercise : https://projecteuler.net/problem=11

I'm currently working on the Project Euler exercise's and I just finished the eleven exercise. It's working fast enough it takes around 15-25 ms to calculate the output, the volume of the code is probably bigger than it should be.I barely used any mathematics in order to find the solution I just don't see the mathematics in the question some people will just need a quick glance and they will have an idea how to solve it using math. I'm looking forward to see answer's concerning the code style, of course how to shorten the code and maybe an overall idea of how should I approach such type of tasks.

Explanation

-
General Info

There are 3 methods CollumnValues, RowValues, DiagonalValues
they return array of type int with 4 values which are the values of
the first 4 adjacent numbers. The parameters are startingIndex
which serve's as a starting point and a boolean variable to determine
whether it should go left/right/top/bottom. In DiagonalValues
method the parameter bool leftDiagonal if it's value is true we
take the following diagonal :


10 11 12 13


14 15 16 17


18 19 20 21


22 23 24 25

If the value is false we take :


10 11 12 13


14 15 16 17


18 19 20 21


22 23 24 25

This one is the one that might not be that accurate so I decide to
show it. The other's are pretty much self explanatory :

• CollumnValues - bool top if it's false we go down, else we go up.

• RowValues - bool right if it's false we go left, else we go right.

There are 2 more methods that probably need explanation -
IsInMinBounds and IsInMaxBounds they both take the same
parameters int number and one more int which is the max or the
min. They're job is to check whether the input number is in the
bounds of the array, for example in order to go left we first need
the starting point to be greater than 2 so we can get values at
indexes : 0, 1, 2, 3. They also

Don't use exceptions for flow control

The purpose of this code is to ignore the sum of a row if an exception is thrown during calculation:

if (!stopRows)
{
    try
    {
        sumOfRows = SumOfRows(i);
    }
    catch (Exception)
    {
        stopRows = true;
    }
}

There are several problems with this approach:

• It's a bad practice to throw generic Exception instead of something more specific

• It's a bad practice to catch generic Exception instead of something more specific

• Exceptions are designed to handle unexpected conditions, not for simple flow control

• An exception is never actually thrown

You can replace the above code with this single line:

sumOfRows = SumOfRows(i);

The try-catch for summing columns and diagonals are bad too.
You should eliminate them.
That won't be as simple as with rows,
because during these operations there may be exceptions.
You need to understand where those exceptions come from: index out of bounds.
The right way to handle these is to check boundaries and thereby eliminate index of bounds exceptions.
I will give you some hints in a later section.

Overcomplicated code

This method is really unnecessarily complicated:

private static int SumOfRows(int currentNumber)
{
    IEnumerable outputSumOfLeftRows = RowValues(currentNumber, false);
    int sumOfLeftRows = outputSumOfLeftRows.Aggregate(1, (current, row) => current * row);
    IEnumerable outputSumOfRightRows = RowValues(currentNumber, true);
    int sumOfRightRows = outputSumOfRightRows.Aggregate(1, (current, row) => current*row);
    return sumOfLeftRows > sumOfRightRows ? sumOfLeftRows : sumOfRightRows;
}

You are calculating products from left, and also from right. But the calculations from left and right will be the same, so it's enough to do one of them: from left only, for example. So we can simplify the method to this:

private static int SumOfRows(int currentNumber)
{
    IEnumerable outputSumOfLeftRows = RowValues(currentNumber, false);
    int sumOfLeftRows = outputSumOfLeftRows.Aggregate(1, (current, row) => current * row);
    return sumOfLeftRows;
}

You can further simplify this by cutting out the unnecessary local variables:

private static int SumOfRows(int currentNumber)
{
    return RowValues(currentNumber, false)
             .Aggregate(1, (current, row) => current * row);
}

And since it's enough to sum from left, you can simplify RowValues accordingly, eliminate the bool right parameter.

You can do similarly for SumOfCollumns:

private static int SumOfCollumns(int currentNumber)
{
    return CollumnValues(currentNumber, true)
             .Aggregate(1,(current, collumn) => current * collumn);
}

And also adjust CollumnValues accordingly.

In case of the diagonals, you do need to calculate from both the left and the right. However, as mentioned earlier, you should check the bounds rather than letting the process end in an index out of bounds exception.

Naming

The method and variables should be improved.
Most notably, the program is about calculating products,
so all the methods and variables with "sum" should be renamed to "product".

Also, column is misspelled as "collumn" at many places.

Finding the max value

You really don't need an int[3] to store the max values of the row, column, diagonal products. A single int and some Math.Max calls could do it. So instead of this:

if (maxSum[0] < sumOfCollumns)
{
    maxSum[0] = sumOfCollumns;
}
if (maxSum[1] < sumOfDiagonals)
{
    maxSum[1] = sumOfDiagonals;
}
if (maxSum[2] < sumOfRows)
{
    maxSum[2] = sumOfRows;
}

You could simplify to this:

max = Math.Max(max, sumOfCollumns);
max = Math.Max(max, sumOfRows);
max = Math.Max(max, sumOfDiagonals);

StackExchange Code Review Q#124660, answer score: 5