johnas
johnas

Reputation: 13

Functions returns false even when the if statement is true

Here is the problem: https://leetcode.com/problems/happy-number/

My solution:

static int count = 0; 
public static void Main(string[] args)
{
    Console.WriteLine(happyNumber(19));
    Console.ReadLine(); 
}

public static bool happyNumber(int  a)
{
    double result = 0;
    Stack<int> stapel = new Stack<int>(); 
    //Split the integer into single digits and save them in a stack
    while (a.ToString().Count() > 1)
    {
        stapel.Push(a % 10);
        a = a / 10;
    }
    if (a.ToString().Count() == 1)
    {
        stapel.Push(a);
    }
    // Add the square of the digits to get the result
    foreach (var item in stapel)
    {
        result += Math.Pow((double)item, 2);
    }
    // Check if it's a happy number
    if(result == 1.0)
    {
        return true;
    }
    // counter to stop if it is a endless loop
    else if(count < 100)
    {
        count++;
        happyNumber((int)result);
    }
    return false;
}

So the input 19 is a happy number and the if clauses is true in the 4th run. You can set a breakpoint at if(result == 1.0) to check it. So why my function returns false instead?

Upvotes: 1

Views: 767

Answers (3)

amcdermott
amcdermott

Reputation: 1585

It happens because your happyNumber method calls itself (3rd last line) and then from this call it hits the return true line - but that only returns one step up the stack to the happyNumber method.... which then hits return false.

Upvotes: 0

Tom Galvin
Tom Galvin

Reputation: 901

You're unnecessarily casting to a double. Make result an int rather than a double (or make it a long if you're concerned the result will be too large for an int). Replace the call to Math.Pow with manually squaring item, like so:

result += item * item;

The reason that control flow does not enter the if(result == 1.0) block is due to the way floating-point values are internally represented. Testing for equality between doubles is problematic, and so (in this scenario) you should probably just avoid using them entirely as they are unneeded.

You also have a recursive call here:

happyNumber((int)result);

However, that call does nothing, as you're not actually doing anything with the return value. Consider replacing that line with:

return happyNumber((int)result);

This will return the value of the recursive call, rather than just discarding it.

Upvotes: 2

Charles Mager
Charles Mager

Reputation: 26213

Your function is recursive, but you don't do anything with the result of the recursive call.

If you change:

happyNumber((int)result);

To:

return happyNumber((int)result);

Then your result for 19 is true. There are potentially other issues with the comparison of floating point numbers, but that's probably your main problem!

Upvotes: 2

Related Questions