DudeWhoWantsToLearn
DudeWhoWantsToLearn

Reputation: 771

Is it okay to exit a loop when an exception is thrown?

I solved a task on Hackerrank.com, where the problem was like this:

You have an Array. This Array contains numbers.
Now you enter two numbers:

  • The first one describes a sum
  • The second one describes the amount of indexes (sequence length) you add together

In the end you get the amount of sequences whose sum is your defined number

For example:

Your array is [ 1, 2, 3, 4], your sum is 3 and your sequence length is 2.
Now you take the first two indexes and output the sum: [1, 2] = 3.
This is equal to your sum, so now you have found one sequence.
The next sequence is [ 2, 3 ] = 5. This is not equal to 3, so your sequence counter stays 1.
The last sequence is [3, 4] = 7. This is also not equal to 3 and in the end, you found one sequence.

I wrote this code for that:

static int GetSequences(List<int> s, int d, int m)
{
    //m = segment-length
    //d = sum

    int count = 0;
    int j = 0;
    int k = 0; 

    do
    {
        try
        {
            List<int> temp = new List<int>();

            for (int i = 0; i < m; i++)
            {
                temp.Add(s[i + k]);
            }
            if (temp.Sum() == d)
            {
                count++;
            }
            j++;
            k++;
        }
        catch (ArgumentOutOfRangeException)
        {
            break;
        }

    } while (true);

    return count;
}

As I didn't know how often I have to count
(For example a 6-Length-Array with a sequence-length of 3 has 4 sequences (1,2,3 | 2,3,4 | 3,4,5 | 4,5,6)),
I am stopping the while loop when the index is out of range. but I'm not sure if this solution is okay. Not just with program speed, but also with code cleanliness. Is this code acceptable, or is it better to use a for loop, which loops for example exactly 4 times for a 6-length array with 3-Length sequences?

Upvotes: 2

Views: 978

Answers (4)

JohnyL
JohnyL

Reputation: 7142

int[] arr = new[] { 1, 2, 1, 2 };
// Sum and len are given by the task.
// 'last' is the last index where we should stop iterating.
int sum = 3, len = 2, last = arr.Length - len;
// One of the overloads of Where accepts index, i.e. the position of element.
// 1) We check that we don't go after our stop-index (last).
// 2) Avoid exception by using '&&'.
// 3) We use C# 8 Range (..) to get the slice of the numbers we need:
//    we start from the current position (index) till then index,
//    calculated as current index + length given by the task.
// 4) Sum all the numbers in the slice (Sum()) and compare it with the target sum,
//    given by the task (== sum).
// 5) The count of all matches (Count()) is the sought amount of sequences.  
int count = arr.Where((z, index) => index <= last && arr[index..(index+len)].Sum() == sum).Count();

Upvotes: 0

canton7
canton7

Reputation: 42245

It would be better to fix your code so that it doesn't routinely throw exceptions:

You sum each of these segments:

0  1  2  3    start = 0
|  |          summing indexes: 0, 1
+--+

0  1  2  3    start = 1
   |  |       summing indexes: 1, 2
   +--+

0  1  2  3    start = 2
      |  |    summing indexes: 2, 3
      +--+

The bracket starts at the index start, and has a size of m. The length of s is given by s.Count. Therefore we want to keep going until start + m == s.Count.

(I always find it's useful to draw these things out, and put sample numbers in, in order to make sure you've got the maths right. In the sample above, you can see that we stop when start (2) + m (2) == the array size (4))

static int GetSequences(List<int> s, int d, int m)
{
    //m = segment-length
    //d = sum

    int count = 0;

    for (int start = 0; start + m <= s.Count; start++)
    {
        List<int> temp = new List<int>();
        for (int i = 0; i < m; i++)
        {
            temp.Add(s[start + i]);
        }
        if (temp.Sum() == d)
        {
            count++;
        }
    }

    return count;
}

However, you can improve your code a bit:

  1. Use meaningful variable names
  2. Don't create a new temporary list each time, just to sum it
  3. Check your inputs
static int GetSequences(List<int> numbers, int targetSum, int segmentLength)
{
    if (numbers == null)
        throw new ArgumentNullException(nameof(numbers));
    if (segmentLength > numbers.Count)
        throw new ArgumentException("segmentLength must be <= numbers.Count");

    int count = 0;

    for (int start = 0; start + segmentLength <= numbers.Count; start++)
    {
        int sum = 0;
        for (int i = 0; i < segmentLength; i++)
        {
            sum += numbers[start + i];
        }
        if (sum == targetSum)
        {
            count++;
        }   
    }
}

Upvotes: 2

Platypus
Platypus

Reputation: 371

Usually except for switch/case there is often no real reason to use break. Also an exception MUST be as the name says exceptional, so it MUST NOT be a part of your logic. As said Jeppe you can use the methods and attributes the framework provides you to do as you like. Here s.Count seems to be the way to go.

Upvotes: 0

John von No Man
John von No Man

Reputation: 3030

It's not recommended, no. Exceptions should be reserved for stuff that isn't supposed to happen, not flow control or validation.

What you want is to use conditional logic (if statements) and the break keyword.

Also, codereview.stackexchange.com is better suited for these kinds of questions.

Upvotes: 5

Related Questions