3
\$\begingroup\$

I started Python 2-3 months ago, but I really haven't done much since then, just little things. Yesterday, I wrote this simple quadratic equation solver. You can give \$a, b\$ and \$c\$ any value, but they must all be integers.

I really want to know what you think about it.

from math import sqrt

while True:
        try:
                a = int(input("\nEnter a : "))
                b = int(input("Enter b : "))
                c = int(input("Enter c : "))
        except:
                print("You can only enter integers")
                continue
        if a > 0:
            sgn_a = ""
            if b > 0:
                sgn_b = "+"
            else:
                sgn_b = "-"
            if c > 0:
                sgn_c = "+"
            else:
                sgn_c = "-"
        else:
            sgn_a = "-"
            if b > 0:
                sgn_b = "+"
            else:
                sgn_b = "-"
            if c > 0:
                sgn_c = "+"
            else:
                sgn_c = "-"

        if a == 0:
            part_a = ""
        elif a == 1:
            part_a = sgn_a+"x"+str(chr(0x00B2))
        else:
            part_a = sgn_a+str(abs(a))+"x"+str(chr(0x00B2))

        if b == 0:
            part_b = ""
        elif b == 1:
            if a != 0:
                part_b = sgn_b+"x"
            else:
                part_b = "x"
        else:
            if a != 0:
                part_b = sgn_b+str(abs(b))+"x"
            else:
                part_b = str(abs(b))+"x"

        if c == 0:
            if a == 0 and b == 0:
                part_c = "0"
            else:
                part_c = ""
        elif a == 0 and b == 0:
            part_c = str(abs(c))
        else:
            part_c = sgn_c+str(abs(c))

        print("\nYour equation is %s%s%s = 0\n" %(part_a,part_b,part_c))
        if a != 0 and b != 0 and c != 0: 
                D = b**2-(4*a*c)
                if D > 0:
                        x_1 = ((-b+sqrt(D))/2*a)
                        x_2 = ((-b-sqrt(D))/2*a)
                        print("X1 = %f\nX2 = %f" %(x_1,x_2))
                elif D == 0:
                        x = -b/(2*a)
                        print("X1 = X2 = %d" %x)
                elif D < 0:
                        x_1 = ((-b+sqrt(-D))/2*a)
                        x_2 = ((-b-sqrt(-D))/2*a)
                        print("X1 = %f%s\nX2 = %f%s" %(x_1,"i",x_2,"i"))
        elif a == 0 and b != 0 and c != 0:
                x = -c/b
                if c % b != 0:
                        print("X = %d/%d or %f" %(-c,b,x))
                else:
                        print("X = %d" %x)
        elif a != 0 and b == 0 and c != 0:
                if -c > 0:
                        x_1 = sqrt(-c/a)
                        x_2 = -x_1
                        print("X1 = %f\nX2 = %f" %(x_1,x_2))
                else:
                        x_1 = sqrt(c/a)
                        x_2 = -x_1
                        print("X1 = %f%s\nX2 = %f%s" %(x_1,"i",x_2,"i"))
        elif a != 0 and b != 0 and c == 0:
                x_2 = -b/a
                if b % a != 0:
                        print("X1 = 0\nX2 = %d/%d or %f" %(-b,a,x_2))
                else:
                        print("X1 = 0\nX2 = %d" %x_2)
        elif a == 0 and b != 0 and c == 0:
                        print("X = 0")
        elif a == 0 and b == 0:
                if c != 0:
                        print(c,"!= 0")
                else:
                        print("0 = 0. DUH!")
\$\endgroup\$
4
\$\begingroup\$

Do not do use bare except

Your execpt catches all exceptions including KeyboardInterrupt. Once the program is launched, there is no way (as far as I can see) to stop it.

If you are interested in failed conversions, you should catch ValueError.

Bug (solutions)

Using these values :

        a = int("0") # int(input("\nEnter a : "))
        b = int("2") # int(input("Enter b : "))
        c = int("3") # int(input("Enter c : "))

I got

Your equation is 2x+3 = 0
X = -3/2 or -2.000000

Another bug (pretty-printing)

With :

        a = int("0") # int(input("\nEnter a : "))
        b = int("-1") # int(input("Enter b : "))
        c = int("-1") # int(input("Enter c : "))

I get :

Your equation is 1x-1 = 0
X = -1

Yet another bug (no solutions)

With :

        a = int("1") # int(input("\nEnter a : "))
        b = int("0") # int(input("Enter b : "))
        c = int("0") # int(input("Enter c : "))

No solution is printed.

Organisation

If you want to make things easier to read/maintain/fix, you should split the logic into multiple independant entities like functions for instance. Once could handle the logic to get the input from the user, one to generate the pretty-printed equations, one to look for solution, etc.

Also, you should try to write automatic tests. I'll let you find online how to write unit tests in Python.

Finally, it is a good habit to define functions/classes in your file but to write code that actually calls it behind a if __name__ == "__main__": test so that it does what you want when you use your file as a script but you can also import it to enjoy the functions and stuff without having any side-effects.

Useless conversions

chr returns a string, there is no need to convert it to string afterward.

Do not repeat yourself

You have :

            if b > 0:
                sgn_b = "+"
            else:
                sgn_b = "-"
            if c > 0:
                sgn_c = "+"
            else:
                sgn_c = "-"

in two different places. There is no need for this, just do this away from the if a > 0: test.

Keep it simple

When looking for a solution, you are trying to handle all scenarios in a pretty akward way. The best place to start with is to answer the simple question : "is a different from 0 ?". If a is 0, next question is "is b different from 0?".

Trying to group the different cases makes things easier to read. Also, it is easier to ensure you've handled all interesting cases.

Suggestion

When you pretty print the equations, 1 is a special case, -1 should probably be considered just as special.

My final version of the code is below. I have mostly moved code from one place to another and I haven't fixed any issue because it is more interesting for your to do it. Also, I have a few ideas to make the pretty-printing function much more concise but it is a bit more advanced and I feel like you should learn one step at a time.

from math import sqrt

def get_input():
    while True:
        try:
            a = int("1") # int(input("\nEnter a : "))
            b = int("0") # int(input("Enter b : "))
            c = int("0") # int(input("Enter c : "))
            return a, b, c
        except ValueError:
                print("You can only enter integers")

def get_pretty_eq(a, b, c):
    if a > 0:
        sgn_a = ""
    else:
        sgn_a = "-"
    if a == 0:
        part_a = ""
    elif a == 1:
        part_a = sgn_a+"x"+chr(0x00B2)
    else:
        part_a = sgn_a+str(abs(a))+"x"+chr(0x00B2)

    if b > 0:
        sgn_b = "+"
    else:
        sgn_b = "-"
    if b == 0:
        part_b = ""
    elif b == 1:
        if a != 0:
            part_b = sgn_b+"x"
        else:
            part_b = "x"
    else:
        if a != 0:
            part_b = sgn_b+str(abs(b))+"x"
        else:
            part_b = str(abs(b))+"x"

    if c > 0:
        sgn_c = "+"
    else:
        sgn_c = "-"
    if c == 0:
        if a == 0 and b == 0:
            part_c = "0"
        else:
            part_c = ""
    elif a == 0 and b == 0:
        part_c = str(abs(c))
    else:
        part_c = sgn_c+str(abs(c))
    return part_a + part_b + part_c + " = 0"

def get_solution_as_string(a, b, c):
    if a != 0:
        if b != 0:
            if c != 0: 
                D = b**2-(4*a*c)
                if D > 0:
                        x_1 = ((-b+sqrt(D))/2*a)
                        x_2 = ((-b-sqrt(D))/2*a)
                        return "X1 = %f\nX2 = %f" %(x_1,x_2)
                elif D == 0:
                        x = -b/(2*a)
                        return "X1 = X2 = %d" %x
                elif D < 0:
                        x_1 = ((-b+sqrt(-D))/2*a)
                        x_2 = ((-b-sqrt(-D))/2*a)
                        return "X1 = %f%s\nX2 = %f%s" %(x_1,"i",x_2,"i")
            else:
                x_2 = -b/a
                if b % a != 0:
                        return "X1 = 0\nX2 = %d/%d or %f" %(-b,a,x_2)
                else:
                        return "X1 = 0\nX2 = %d" %x_2
        else:
            if c != 0:
                if -c > 0:
                        x_1 = sqrt(-c/a)
                        x_2 = -x_1
                        return "X1 = %f\nX2 = %f" %(x_1,x_2)
                else:
                        x_1 = sqrt(c/a)
                        x_2 = -x_1
                        return "X1 = %f%s\nX2 = %f%s" %(x_1,"i",x_2,"i")
            # this lack of "else" case is very suspicious
    else:
        if b != 0:
            if c != 0:
                x = -c/b
                if c % b != 0:
                    return "X = %d/%d or %f" %(-c,b,x)
                else:
                    return "X = %d" %x
            else:
                return "X = 0"
        else:
            if c != 0:
                return c,"!= 0"
            else:
                return "0 = 0. DUH!"

if __name__ == "__main__":
    a, b, c = get_input()
    print("\nYour equation is %s\n" % get_pretty_eq(a, b, c))
    print(get_solution_as_string(a, b, c))
\$\endgroup\$
  • \$\begingroup\$ Your corrected version has 2 bugs when the roots are complex and all coefficients are non-zero. Your code will always print purely imaginary roots, but the roots should have real part -b/(2 * a) and imaginary part ±sqrt(D)/(2 * a). There's also a problem with the order of operations where you've written x_1 = ((-b+sqrt(-D))/2*a). x/2*a is equivalent to (x/2) * a. You need parentheses around the 2*a. \$\endgroup\$ – Kyle Jul 20 '15 at 13:38
  • \$\begingroup\$ Are these issues in the original code ? I haven't changed much things but I might have done it wrong anyway. \$\endgroup\$ – SylvainD Jul 20 '15 at 13:40
  • \$\begingroup\$ I probably should have checked that, but indeed they are. \$\endgroup\$ – Kyle Jul 20 '15 at 13:42
  • 1
    \$\begingroup\$ Great, thanks! Because I had found many issues too, I thought it was more interesting to point them out and to give general advices rather than fixing the problems ("give a man a fish ..."). \$\endgroup\$ – SylvainD Jul 20 '15 at 13:43
2
\$\begingroup\$

Use if __name__ == "__main__":

This:

        if a > 0:
            sgn_a = ""
            if b > 0:
                sgn_b = "+"
            else:
                sgn_b = "-"
            if c > 0:
                sgn_c = "+"
            else:
                sgn_c = "-"
        else:
            sgn_a = "-"
            if b > 0:
                sgn_b = "+"
            else:
                sgn_b = "-"
            if c > 0:
                sgn_c = "+"
            else:
                sgn_c = "-"

can be shortened to

        if a > 0:
            sgn_a = ""
        else:
            sgn_a = "-"
        if b > 0:
            sgn_b = "+"
        else:
            sgn_b = "-"
        if c > 0:
            sgn_c = "+"
        else:
            sgn_c = "-"

There also seems to be an extra indentation (4 spaces) at the start of every line. I'm guessing that it is caused when copy-pasting the code.

Also, it is encouraged to use .format() instead of % for formatting strings.

\$\endgroup\$
  • 1
    \$\begingroup\$ How did I not catch that? \$\endgroup\$ – Emphirus Jul 20 '15 at 9:22

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.