1
\$\begingroup\$

Here is my code, which doesn't seem to work for numbers in between -1 and 0 (works perfectly fine otherwise)

I think the code enters an infinite loop, somehow, hence I get no result after entering the number. Please have a look and let me know what changes should be made. I am a beginner at programming and would appreciate any help!

    cube = float(input('Enter:'))
if abs(cube) > 1:
    num_guesses = 0
    epsilon = 0.0001
    low = 0
    high = cube
    guess = (high + low) / 2.0

    while abs(abs(guess) ** 3 - abs(cube)) >= epsilon:
        if cube > 0:
            if guess ** 3 < cube:
                low = guess
            else:
                high = guess
            guess = (low + high) / 2.0
            num_guesses += 1
        else:
            if guess ** 3 > cube:
                low = guess
            else:
                high = guess
            guess = (low + high) / 2.0
            num_guesses += 1

if abs(cube) < 1:
    num_guesses = 0
    epsilon = 0.0001
    low = cube
    high = 1
    guess = (high + low) / 2.0

    while abs(abs(guess) ** 3 - abs(cube)) >= epsilon:
        if cube > 0:
            if guess ** 3 < cube:
                low = guess
            else:
                high = guess
            guess = (low + high) / 2.0
            num_guesses += 1
        else:
            low = -1
            high = cube
            if guess ** 3 > cube:
                high = guess
            else:
                low = guess
            guess = (low + high) / 2.0
            num_guesses += 1


print(num_guesses)
print('The cube root of',cube,'is closest to',guess)

Thanks a lot!

\$\endgroup\$

closed as off-topic by Toby Speight, AlexV, dfhwze, t3chb0t, ferada Jul 5 at 12:03

This question appears to be off-topic. The users who voted to close gave this specific reason:

  • "Code not implemented or not working as intended: Code Review is a community where programmers peer-review your working code to address issues such as security, maintainability, performance, and scalability. We require that the code be working correctly, to the best of the author's knowledge, before proceeding with a review." – Toby Speight, AlexV, dfhwze, t3chb0t, ferada
If this question can be reworded to fit the rules in the help center, please edit the question.

  • \$\begingroup\$ Welcome to Code Review! Your code has to be working as intended before it is ready to be reviewed. Code that is not yet working as intended is off-topic here. Stack Overflow is usually a more appropriate place to ask for help on how to solve such issues. Once everything works, come back and we will be happy to provide feedback on how it can be improved. \$\endgroup\$ – AlexV Jul 4 at 11:03
  • \$\begingroup\$ Okay, thank you \$\endgroup\$ – arya_stark Jul 4 at 11:10
  • \$\begingroup\$ @arya_stark, try to use only one while loop ; that should actually make the entire problem simpler. In addition try to start with high = 1 + abs(cube) and low = -high. You guess is correct, and the update of low and high also seems correct. \$\endgroup\$ – Pål GD Jul 4 at 12:10
1
\$\begingroup\$

There are some problems with your code, the algorithm being (too complicated, it is also) wrong.

In addition to that, you haven't made any functions, which you should strongly consider doing.

The better way of writing this script is to write two (or more) functions, one function being the actual cuberoot function which accepts one (or two) argument, namely the number you want to find the cube root for. It should return one number, the cube root.

Writing functionality in function, or perhaps encapsulating functionality in functions allows you to more easily test the function. Having all functionality in functions, also allows you to import cuberoot from a different file. This is essential when building more complex applications and libraries later.

I have attached one example of how you can modularize your code so that it is both easy to test and easy to import, and not least of all, it's easy to read and thus understand.

As you can see in the while loop, it is quite close to your example, but a bit simplified.

I have also added an assert statetment in the main function to easily verify that the result is correct. Adding automatic tests are now quite easy as well.

The bottommost __main__ part is there to ensure that the main function is called only if we call the file as a script:

$ python cuberoot.py -0.5
-0.7936859130859375

Here is the refactored code:

def cuberoot(cube, epsilon=0.0001):
    """Compute cube root of n using binary search.

    We yield the result when |result**3-n|<epsilon.
    """
    high = 2 + abs(cube)
    low = -high
    while True:
        guess = (low + high) / 2.0
        err = abs(cube - guess ** 3)
        if err < epsilon:
            return guess
        if guess ** 3 > cube:
            high = guess
        else:
            low = guess


def main():
    """Read one number from command line and print its cube root."""
    from sys import argv

    try:
        cube = float(argv[1])
    except ValueError:
        exit("Usage: cuberoot n [n must be a number]")
    except IndexError:
        exit("Usage: cuberoot n")

    cr = cuberoot(cube)
    assert abs(cr ** 3 - cube) < 0.0001  # verify the result
    print(cr)


if __name__ == "__main__":
    main()
\$\endgroup\$

Not the answer you're looking for? Browse other questions tagged or ask your own question.