2
\$\begingroup\$

This is a my first Python program, and whilst I am new to Python I would like to keep in good practice

Below is a short program to work out what type a triangle is and if it makes a valid triangle

I have tried to use as little documention for this as possible to try and get used to how things work in Python so I can only imagine the mistakes I have made. However please do mention anything that should have be done better

# Determin if triangle is Scalene. Isosceles or equilateral
# Also works out if lengths can make a triangle

from decimal import *

getcontext().prec = 3
getcontext().rounding = ROUND_HALF_UP

#Needs to be divided to re-set decimal place I think
a = Decimal(input("Length of side a = ")) / 1
b = Decimal(input("Length of side b = ")) / 1
c = Decimal(input("Length of side c = ")) / 1

if a != b and b != c and a != c:
    print("This is a a Scalene triangle")
    triangle_type = 'Scalene'
elif a == b and c == b:
    print("This is an Equilateral triangle")
    triangle_type = 'Equilateral'
else:
    print("This is an Isosceles triangle")
    triangle_type = 'Isosceles'

def is_valid_triangle(a, b, c,triangle_type):
    if triangle_type == 'Equilateral':
        return True #all same lengths will be a valid triangle
    elif triangle_type == 'Isosceles' or triangle_type == 'Scalene':
        if a == b:
            return a + b > c
        elif b == c:
            return b + c > a
        elif a == c:
            return a + c > b
        else: #This will be the scalene triangle
            return a + b > c
    else:
        return False #Message is unclear as could be lengths are negitive or correct int type not used

print('Is this a valid triangle?', is_valid_triangle(a,b,c,triangle_type))
\$\endgroup\$
1
  • 1
    \$\begingroup\$ "so I can only imagine the mistakes I have made" Does it work for your test cases? \$\endgroup\$ – πάντα ῥεῖ Jul 24 '19 at 16:55
3
\$\begingroup\$

Add a __name__ == "__main__" guard, and move the logic into a function separate from the I/O:

def triangle_type(a, b, c):
    '''Return a string indicating the type of triangle
       (Equilateral, Isosceles, Scalene, Impossible)
    '''
    # implementation here...

def main():
    getcontext().prec = 3
    getcontext().rounding = ROUND_HALF_UP

    #Needs to be divided to re-set decimal place I think
    a = Decimal(input("Length of side a = ")) / 1
    b = Decimal(input("Length of side b = ")) / 1
    c = Decimal(input("Length of side c = ")) / 1

    print(f"This is a {triangle_type(a, b, c)} triangle")

if __name__ == "__main__":
    main()

In the implementation, we can save a lot of "or" tests by sorting the lengths before we start:

a, b, c = sorted([a, b, c])

if a + b <= c:
    # N.B. automatically catches a < 0, since b <= c
    return 'Impossible'
if a != b != c:
    return 'Scalene'
elif a == c:
    return 'Equilateral'
else:
    return 'Isosceles'

Modified code

def triangle_type(a, b, c):
    '''
    Return a string indicating the type of triangle
    (Equilateral, Isosceles, Scalene, Impossible)
    '''

    a, b, c = sorted([a, b, c])

    if a + b <= c:
        return 'Impossible'
    if a != b != c:
        return 'Scalene'
    if a == c:
        return 'Equilateral'
    return 'Isosceles'
    
def main():
    a = input("Length of side a: ")
    b = input("Length of side b: ")
    c = input("Length of side c: ")
    print(f"({a}, {b}, {c}) is a {triangle_type(a, b, c)} triangle")

if __name__ == "__main__":
    main()

Further improvement

Use the doctest module to write the tests:

def triangle_type(a, b, c):
    '''
    Return a string indicating the type of triangle
    (Equilateral, Isosceles, Scalene, Impossible)

    >>> triangle_type(1, 1, 2)
    'Impossible'
    >>> triangle_type(-1, -1, -1)
    'Impossible'
    >>> triangle_type(1, 1.0, 1)
    'Equilateral'
    >>> triangle_type(1, 2, 2)
    'Isosceles'
    >>> triangle_type(2, 3, 2)
    'Isosceles'
    >>> triangle_type(2, 3, 4)
    'Scalene'
    '''

    a, b, c = sorted([a, b, c])

    if a + b <= c:
        return 'Impossible'
    if a != b != c:
        return 'Scalene'
    if a == c:
        return 'Equilateral'
    return 'Isosceles'

if __name__ == "__main__":
    import doctest
    doctest.testmod()
\$\endgroup\$
1
  • \$\begingroup\$ Thanks for your answers, I have marked this one as accepted as it contains the more useful information, however both answers provide good points that I need to improve upon \$\endgroup\$ – Mr.Burns Jul 26 '19 at 9:25
2
\$\begingroup\$

A bug (or poorly-specified behaviour):

If we enter an invalid triangle (e.g. 1, 2, 4), the program reports that it's scalene, before telling us that it's not a valid triangle. That's a contradiction - if it's not a triangle, it cannot be a scalene triangle!

I recommend performing the is_valid_triangle() test first, and only continuing to classify the triangle if the test is successful.

And don't forget that valid triangles have three positive sides - any negative values should fail the validity test.

\$\endgroup\$
3
  • \$\begingroup\$ little suggestion to test validity of triangle, valid triangles have three positive sides and sum of two side is greater than 3rd side \$\endgroup\$ – sahasrara62 Jul 26 '19 at 8:36
  • 1
    \$\begingroup\$ @prashant, the posted code already covers the second part of that test; I'm just drawing attention to the missing part. I hope that's clear! \$\endgroup\$ – Toby Speight Jul 26 '19 at 8:52
  • \$\begingroup\$ didn't went throuh the whole code, my mistake, thanks for commenting out \$\endgroup\$ – sahasrara62 Jul 26 '19 at 9:06

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.