1

I was doing a past paper question at school to adapt some pseudo code which checked if an input ISBN number was valid or not, into C#. I have typed the code in, yet it throws an IndexOutOfRangeException error. I am unsure how that error occurred, as this is my first time using int arrays.

I checked that the index is not negative, and I think the maximum index on the list is less than the list size (I tried changing "Count < 13" in the for loop to "Count <= 12", but to no avail).

 using System;
    using System.Collections.Generic;
    using System.Linq;
    using System.Text;
    using System.Threading.Tasks;

namespace ISBNExercise
{
    class Program
    {
        static void Main(string[] args)
        {
            int[] ISBN;
            int Count;
            ISBN = new int[13];
            for(Count = 0; Count < 13; Count++)
            {
                Console.WriteLine("Please enter next digit of ISBN");
                ISBN[Count] = Convert.ToInt32(Console.ReadLine());
            }
            int CalculatedDigit = 0;
            int count = 1;
            while (count < 13)
            {
                CalculatedDigit = CalculatedDigit + ISBN[Count]; //This is where the error is being thrown.
                Count += 1;
                CalculatedDigit = CalculatedDigit + ISBN[Count] * 3;
                Count += 1;
            }
            while (CalculatedDigit >= 10)
            {
                CalculatedDigit -= 10;
            }
            CalculatedDigit = 10 - CalculatedDigit;
            if (CalculatedDigit == 10)
            {
                CalculatedDigit = 0;
            }
            if (CalculatedDigit == ISBN[13])
            {
                Console.WriteLine("Valid ISBN");
            }
            else
            {
                Console.WriteLine("Invalid ISBN");
            }
        }
    }
}
Pinguin
  • 11
  • 2

2 Answers2

1

The problem is you are using the Count instead of count where your error is occurring.

I would suggest naming your variables differently and sticking to the C# convention of using all lower camel case names for variables. This would have resulted in a compiler error that would have illuminated your issue right away because you would be trying to redefine count which you had already definied above.

rory.ap
  • 34,009
  • 10
  • 83
  • 174
1

Your problem is on this line

if (CalculatedDigit == ISBN[13])

You define ISBN with a length of 13 which means the highest index is 12 because the indexes are zero based.

But even before that your while loop is checking count, but you are using Count inside the loop which already has the value of 13 at that point.

One final point your while loop is summing up the 2nd through 13th characters (indexes 1 - 12) and comparing the check digit to presumably the last digit. I'm guessing you actually want to sum the first 12 digits, so you need to set count = 0 and check for count < 12.

int CalculatedDigit = 0;
int count = 0;
while (count < 12)
{
    CalculatedDigit = CalculatedDigit + ISBN[count]; //This is where the error is being thrown.
    count += 1;
    CalculatedDigit = CalculatedDigit + ISBN[count] * 3;
    count += 1;
}

or you can just do the following

int CalculatedDigit = ISBN.Take(12)
                          .Select((v,i) => i%2 == 0 ? v : v*3)
                          .Sum();           
CalculatedDigit = 10 - (CalculatedDigit % 10);
CalculatedDigit = CalculatedDigit == 10 ? 0 : CalculatedDigit;

Your loop that keeps subtracting 10 is really just a modulo (%).

juharr
  • 31,741
  • 4
  • 58
  • 93
  • Thanks! I changed the length to 12 and replaced "< 13" to "<= 12" and it now works perfectly. – Pinguin Oct 23 '15 at 12:10
  • @Pinguin I'm not sure I follow. `< 13` and `<= 12` when incrementing are basically the same. See my edit for what I'm guessing is a better way to calculate your check digit, unless you're saying the ISBN is actually only 12 digits instead of 13. – juharr Oct 23 '15 at 12:14
  • Sorry I said it worked perfectly; it was because the error is gone. But it was marking all ISBN numbers invalid. – Pinguin Oct 23 '15 at 12:27
  • I set count to 0 and now an IndexOutOfRangeException error is being thrown at calculateddigit = calculateddigit + ISBN[count] * 3 – Pinguin Oct 23 '15 at 12:47
  • @Pinguin I've updated my answer with an example of how to change your code and fixed my version that uses Linq to multiply the "odd indexed" digits by 3. – juharr Oct 23 '15 at 15:25