4
\$\begingroup\$

The first code I ever wrote, sorry that the prints are in German. I tried to do some code improvements myself but well whenever I do I fail. Hopefully somebody can help me improve my coding skills.

while True:
    print(" ")
    print("ax^2+bx+c ausrechen UwU, Bitte Achte!!! Bei Kommazahlen . benutzen, nicht , Beispiel: 0.5 0.7 1.6")
    import time

    print("Auf ein Neues?")

    U1 = input("input a ")
    I1 = input("input b ")
    O1 = input("input c ")
    if (U1.isalpha() or I1.isalpha() or O1.isalpha()) is False:
        print("doing some math")
        a = float(U1)
        b = float(I1)
        c = float(O1)
        if ((b * b) - (4 * a * c)) > 0:
            q = float(((b * b) - (4 * a * c)) ** (1 / 2))
            x = float(2 * a)
            y = float(-1 * b)
            z = float(q * (-1))
            result1 = str((y + q) / x)
            result2 = str((y + z) / x)

            print("Nullstelle1=" " " + result1 + "")
            print("Nullstelle2=" " " + result2 + "")

            abc = float((-1 * b) / (2 * a))
            Moo = float((4 * a * c) - (b ** 2)) / (4 * a)

            cba = str(abc)
            oom = str(Moo)

            print("Scheitelpunkt: (" + cba + "|" + oom + ")")

            if (a > 0) is True:
                print("Nach oben geöffnete Funktion")
                print("Schnittpunkt mit der Y-Achse ist" " " + O1 + " ")

            else:
                print("Nach unten geöffnete Funktion")
                print("Schnittpunkt mit der Y-Achse ist" " " + O1 + " ")

        else:
            print("sorry, geht nicht da eine negative zahl unter der wurzel sein würde")
            time.sleep(3)
    else:
        print("nur Zahlen bitte")
\$\endgroup\$
  • 1
    \$\begingroup\$ (b * b) - (4 * a * c) == 0 is actually also fine, your two Nullstellen are just the same point. So it should be (b * b) - (4 * a * c) >= 0. \$\endgroup\$ – Graipher May 20 '19 at 14:34
3
\$\begingroup\$

welcome to code review!

The first thing I want to do, is to put the code into a function. This separates it from anything else you might run, and allows you more control. I've used the name quadratic_solver and instead of taking input(), it now takes three arguments which are the coefficients. I've also added a docstring which briefly explains the function's purpose.

My German's not very good so I've just left your comments in; apologies if they're now incorrect. As per PEP8 (python's style guide), I've put your import before the function at the start of the file.

import time


def quadratic_solver(a, b, c):
    """
    Solves quadratic equations of the form ax**2 + bx + c
    """

    print("ax^2+bx+c ausrechen UwU, Bitte Achte!!! Bei Kommazahlen . benutzen, nicht , Beispiel: 0.5 0.7 1.6")

...

Now, if you want to check the type of your arguments you can do a simple:

type(val) is int or type(val) is str for example.

To loop over several values we can use a generator. Here, we loop over all values in a list [a, b, c] and ensure they are NOT all ints:

if not all(type(coeff) is int for coeff in [a, b, c]):

I'm not 100% sure why you're checking the type but perhaps your inputs are delicate in some way.

The rest of the code is fairly unchanged. I have however removed a lot of the casting to floats and strings.

If you want to concatenate strings with variables, you can use an fstring. This is of the form:

print(f"Nullstelle1= {result1}")

Where any variables are given in {} and an f at the start of the string denotes to put the variable inside the string as a string.

fstrings can also contain code, so I've used a ternary operator instead of your if else statement:

print(f"Nach {'oben' if a > 0 else 'unten'} geöffnete Funktion")

I've eliminated a few extra variables and brackets too, just for neatness. I'd recommend changing your variable names perhaps to something clearer, and adding a few comments to show what's going on.

Putting this all together, we get:

import time


def quadratic_solver(a, b, c):
    """
    Solves quadratic equations of the form ax**2 + bx + c
    """

    print("ax^2+bx+c ausrechen UwU, Bitte Achte!!! Bei Kommazahlen . benutzen, nicht , Beispiel: 0.5 0.7 1.6")

    if not all(type(val) is int for val in [a, b, c]):
        print("doing some math")

        numer = b * b - 4 * a * c
        if numer > 0:
            q = numer ** (1 / 2)

            result1 = (-b + q) / (2 * a)
            result2 = (-b - q) / (2 * a)

            print(f"Nullstelle1= {result1}")
            print(f"Nullstelle2= {result2}")

            abc = -b / (2 * a)
            moo = (4 * a * c - b ** 2) / (4 * a)

            print(f"Scheitelpunkt: ({abc}|{moo})")

            print(f"Nach {'oben' if a > 0 else 'unten'} geöffnete Funktion")
            print(f"Schnittpunkt mit der Y-Achse ist {c}")

        else:
            print("sorry, geht nicht da eine negative zahl unter der wurzel sein würde")
            time.sleep(3)
    else:
        print("nur Zahlen bitte")

Finally, to call your function, you can now put the following:

if __name__ == '__main__':
    quadratic_solver(1.0, 2.0, 3.0)

Wrapping the function in the if __name__ == '__main__': ensures the function is being run from the right place and not externally. 1.0, 2.0 and 3.0 are just some example coefficients.

I would also consider removing the time.sleep(3); what is it for?

EDIT:

For the type checking, per @AlexV's comment, if you're checking an input is of a valid type, it's better to check it IS that type, rather than ISN'T another type. So the condition would actually become:

if all(type(coeff) is float for coeff in [a, b, c]):

You could even use Python's new type hinting if you like:

def quadratic_solver(a: float, b: float, c: float):
    ...

```
\$\endgroup\$
  • 1
    \$\begingroup\$ The "type checking" is performed on the user input to make sure it's just numbers, so it's more of an input validation. I think the OP introduced this to avoid exceptions during conversion with float(...). \$\endgroup\$ – AlexV May 20 '19 at 13:42
  • \$\begingroup\$ Added an edit to better explain type checking, thanks. \$\endgroup\$ – QuantumChris May 20 '19 at 13:48
  • \$\begingroup\$ Excuse me if I've been misleading, but the original input validation implemented by the OP steps in earlier in the process. It tries to make sure that the input given by the user on the command line, which is always a string in Python 3, can actually be interpreted as a float. Your recommendations are nevertheless valid on their own right. \$\endgroup\$ – AlexV May 20 '19 at 13:58
3
\$\begingroup\$

@HoboProber already gave a good answer talking about various improvements to be done on the code in general, so let's have a closer look at the core algorithm.


Your implementation uses the standard form for quadratic equations

$$a \cdot x^2+b \cdot x + c = 0$$

and implements the well known quadratic formula to solve and analyze them.

$$ x_{1,2} = \frac{-b \pm \sqrt{b^2 - 4ac}}{2a} $$

The term under the square root is often called discriminant in English literature. Also, Python has a builtin math module with all kind of useful mathematical functions, e.g. math.sqrt(...), so there is no need to tinker with ... ** (1/2).

With that (and @Graipher's comment) in mind, you should go from

if ((b * b) - (4 * a * c)) > 0:
    q = float(((b * b) - (4 * a * c)) ** (1 / 2))

to

import math   # at the beginning of the script

...   # other code here

discriminant = (b * b) - (4 * a * c)
if discriminant >= 0:
    q = math.sqrt(discriminant)

Considering best-practices for programmers in general, and the already mentioned Style Guide for Python Code, there are also some other variable names to be improved, especially Moo and abc since they are quite generic. In your case the coefficients a, b, and c would be exempt from that rule, since they have a quite well defined mathematical meaning in this context.

Just be aware that relying on mathematical conventions can have "unwanted" side effects, since this will often mean others always try to see the mathematical meaning behind, possibly arbitrary named, single letter variables. That might lead to some confusion in your case since you're using q, which is also often used in an alternative formulation of the problem (see Wikipedia - Reduced quadratic equation), but with a different meaning.


Another thing I would like to tell you about is Unicode support in Python. All strings in Python 3 are Unicode by default. That allows you to use something like

print("a·x² + b·x + c = 0")

directly in your code to generate a more visually appealing command line output.


Python also has the notion of "It's easier to ask for forgiveness than it is to get permission". In your context that would apply to the input validation section, which is a good thing to do! I just want to present an alternative approach to you.

At the moment your code does the following:

...
U1 = input("input a ")
I1 = input("input b ")
O1 = input("input c ")
if (U1.isalpha() or I1.isalpha() or O1.isalpha()) is False:
    print("doing some math")
    a = float(U1)
    b = float(I1)
    c = float(O1)
    ...
else:
    print("nur Zahlen bitte")

Here, you are trying to avoid situations where a conversion from string to float might raise an exception. So that would be the "asking for permission" part. The "ask for forgiveness"-way could be like

...
input_a = input("input a ")
input_b = input("input b ")
input_c = input("input c ")
try:
    a = float(input_a)
    b = float(input_b)
    c = float(input_c)
except ValueError:
    # float will throw an ValueError if it cannout convert the input
    print("nur Zahlen bitte")
else:
    # you get here only if no ValueError was raised
    print("doing some math")
    quadratic_solver(a, b, c)   # this might be the function as proposed by HoboProber
...

This approach actually tries to convert the given user input to a float and handles the case where it fails. In that way, it's often considered more direct and clearer on what you're trying to accomplish. In theory, there would also be no problem to not use the else block of try-except and simply put it in the try block. The significant disadvantage of the second approach is that also every value error that might be raised in further computations would trigger print("nur Zahlen bitte"), and therefore make it harder to find out what caused the problem in the first place. Some general recommendations for catching exceptions are:

  1. try-except as narrow as you can! This is a generalization of the statement above and means to only surround the parts you are willing to handle with a try-except structure. Otherwise debugging might get quite a bit harder.
  2. Only catch what you expect! Do not use except without specifying which exceptions should be handled unless you are a 100% sure that you absolutely don't care what's happening in that try-except. This blank except also catches keyboard interrupts using Ctrl+C, which can be, ummm, unconvenient sometimes.
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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