7
\$\begingroup\$

This is a challenge from Think Python 2nd Edition:

Fermat’s Last Theorem says that there are no positive integers a, b, and c such that aⁿ + bⁿ = cⁿ. for any values of n greater than 2.

  1. Write a function named check_fermat that takes four parameters; a, b, c and n. And that checks to see if Fermat’s theorem holds. If n is greater than 2 and it turns out to be true that aⁿ + bⁿ = cⁿ the program should print, “Holy smokes, Fermat was wrong!” Otherwise the program should print, “No, that doesn’t work.”

  2. Write a function that prompts the user to input values for a, b, c and n, converts them to integers, and uses check_fermat to check whether they violate Fermat’s theorem.

That's what I've done so far. Are there any improvements I should make? And are there any 'best practices' among programmers that I'm missing? I'm just starting out and I'd like to avoid bad habits.

def check_fermat(a, b, c, n):
  if n > 2 and a**n + b**n == c**n:
    print("Holy smokes! Fermat was wrong!")
  elif n <= 2:
    print("The exponent should be grater than '2'")
  else:
    print("No, that doesn't work.")

def check_numbers():
    a = int(input("Choose a number for 'a': "))
    b = int(input("Choose a number for 'b': "))
    c = int(input("Choose a number for 'c': "))
    n = int(input("Choose a number for 'n' that's greater than '2': "))
    check_fermat(a, b, c, n)

check_numbers()
\$\endgroup\$

2 Answers 2

6
\$\begingroup\$

Scripts should use a main guard. This supports the ability to cleanly import the code defined in the script and therefore supports testing, debugging, and development.

Validate input. People make data-entry mistakes, and computers should accomodate humans -- not the other way around. The simplest, most flexible technique is a while-true loop: get the input; if it can be converted and is valid, return the value; otherwise stay in the loop.

Collections are often more powerful than discrete variables.. In your main function, the separate variables (a, b, etc) aren't doing much for you, other than playing a minor documentation role. It might be overkill for an assignment this basic, but if you're trying to be extra diligent, you could take a more data-centric approach: define the input prompts as data and then use that data to drive the collection of the needed integers. This type of data-centric thinking is one of the keys to developing real skill at writing code.

A mathematical function should be data-oriented. Your current check_fermat() is a mathematical function that prints. That's the wrong approach -- at least if we're striving for best-practice coding. A function like this should return a boolean or raise on exception for invalid inputs. Leave the printing to another part of the program -- the one handling input/output for the user.

More data-oriented thinking. Continuing with the theme, one could define the messages to be printed for the user in a collection, allowing us to select the right message based on the return behavior of check_fermat(). Again, well-chosen data can drive the logic and, quite often, reduce, simplify, and clarify the algorithmic code.

PROMPTS = (
    "Choose a number for 'a'",
    "Choose a number for 'b'",
    "Choose a number for 'c'",
    "Choose a number for 'n' that's greater than '2'",
)

MESSAGES = {
    True: "No, that doesn't work.",
    False: "Holy smokes! Fermat was wrong!",
    'LOW': "The exponent should be greater than '2'",
}

def main():
    args = [get_int(p) for p in PROMPTS]
    try:
        print(MESSAGES[check_fermat(*args)])
    except ValueError as e:
        print(e)

def get_int(prompt):
    while True:
        try:
            return int(input(prompt + ': '))
        except ValueError:
            print('Invalid input: try again')

def check_fermat(a, b, c, n):
    if n > 2:
        return a**n + b**n != c**n
    else:
        raise ValueError(MESSAGES['LOW'])

if __name__ == '__main__':
    main()

\$\endgroup\$
1
\$\begingroup\$

Most Fermat code seems really inefficient to me.

How about computing aⁿ+bⁿ and then seeing if the nth root of that number is an integer?

Wouldn't that be a lot more efficient than blindly running all those comparisons?

Maybe something like:

c = (a**n+b**n)**(1/n)      
if isinstance(c, int):
      print('Fermat disproved!')
\$\endgroup\$
4
  • \$\begingroup\$ Hello and welcome to Code Review! Your feedback is highly valued and appreciated! If you would, please elaborate and use code snippets to demonstrate how your solution could be implemented. Thank you! \$\endgroup\$
    – MadHatter
    Mar 17, 2022 at 4:34
  • \$\begingroup\$ Your answer could be improved with additional supporting information. Please edit to add further details, such as citations or documentation, so that others can confirm that your answer is correct. You can find more information on how to write good answers in the help center. \$\endgroup\$
    – Community Bot
    Mar 17, 2022 at 4:38
  • \$\begingroup\$ if isinstance(c, int): \$\endgroup\$
    – gerald
    Mar 18, 2022 at 10:19
  • \$\begingroup\$ Thanks for your suggestions and edits. \$\endgroup\$
    – gerald
    Mar 20, 2022 at 14:32

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

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