1

I have been trying to code to find the LCM of given Array elements. My code looks like this

public long LcmOfArray(List<int> a)
{
    long LCM = 1;
    bool divisible = false;
    int divisor = 2, count = 0;

    while (count != a.Count)
    {
        for (int i = 0; i < a.Count; i++)
        {
            if (a[i] == 0)
            {
                return 0;
            }
            if (a[i] < 0)
            {
                a[i] *= -1;
            }
            if (a[i] % divisor == 0)
            {
                divisible = true;
                a[i] /= divisor;
            }
            if (a[i] == 1)
            {
                count++;
            }
        }

        if (divisible)
        {
            LCM *= divisor;
        }
        else
        {
            divisor++;
        }
    }
    
    return LCM;
}

My problem is that the output console freeze as I enter the input. I have tried another method without the while loop. That is, eliminated while loop and added an if loop in the end.

if (count == a.Count)
{
    return LCM;
}

But now compiler throws an error stating that not all code path returns a value. Can somebody help me with what is wrong in my code? I am a beginner in coding. Thanks in advance!!

Saranya
  • 69
  • 1
  • 4
  • 1
    `bool divisible=false;` That doesn't look to be in the right place. – dxiv Jun 22 '20 at 03:03
  • 1
    Since you say you are new to development, I would recommend making debugging a friend of yours. Go one line at a time and see what's going on with your variables and where it's getting stuck in infinite loop – Jawad Jun 22 '20 at 03:32

2 Answers2

2

You should place bool divisible = false; and int count = 0; in the while loop. And since you can't place int count = 0; out of the while loop you should use while (true) instead of while (count != a.Count) and place the following if statement in the last of the while loop.

if (count == a.Count)
{
    return LCM;
}

Here's the full method :

public static long LcmOfArray(List<int> a)
{
    long LCM = 1;
    int divisor = 2;

    while (true)
    {
        int count = 0;
        bool divisible = false;
        for (int i = 0; i < a.Count; i++)
        {
            if (a[i] == 0)
            {
                return 0;
            }
            if (a[i] < 0)
            {
                a[i] *= -1;
            }
            if (a[i] == 1)
            {
                count++;
            }
            if (a[i] % divisor == 0)
            {
                divisible = true;
                a[i] /= divisor;
            }
        }
        if (divisible)
        {
            LCM *= divisor;
        }
        else
        {
            divisor++;
        } 
        if (count == a.Count)
        {
            return LCM;
        }
    }
}
0

A while loop will run constantly until you give it something to break the loop. In your case, the only way it will ever break the loop appears to be in the scenario in which every item of the List is a 1 or it reaches a 0, so depending on the list, it may or may not get stuck in an infinite loop. For instance, a[i] = 2 appears to create an infinite loop. The other compiler error is because some conditions could skip a return statement.

if(count==a.Count)
{
   return LCM;
}
return void; // because what if count does not equal a.Count?
RyanJMcGowan
  • 1,485
  • 1
  • 15
  • 33