3
\$\begingroup\$

Below is the code I wrote to perform basic Trigonometry without using the math module(except for 1 place) for the purpose of teaching myself basic trig and to improve my python. I have never taken a trig class, so please consider this fact when explaining some fallacies in my trig logic, and please help me correct it.

I would like feedback on

  1. The architecture of the code. I find my use of dictionaries handling the return of data very restrictive and would like to know how better to handle output or returning of values.
  2. The usage of **kargs, is this proper utilization? Is there a different way to write these functions where I can input large amounts of (undermined) data?
  3. Lines 47-64, better ways to write the block of if/elif statements for readability and functionality.
#!/usr/bin/env python3

import math


def sin(*args, **kwargs):
    degrees = kwargs.get('degrees', None)
    opposite = kwargs.get('opposite', None)
    hypotenuse = kwargs.get('hypotenuse', None)

    if degrees:
        return math.sin(math.radians(degrees))
    elif opposite and hypotenuse:
        return(opposite / hypotenuse)


def cos(*args, **kwargs):
    degrees = kwargs.get('degrees', None)
    adjacent = kwargs.get('adjacent', None)
    hypotenuse = kwargs.get('hypotenuse', None)

    if degrees:
        return math.cos(math.radians(degrees))
    elif adjacent and hypotenuse:
        return(adjacent / hypotenuse)


def tan(*args, **kwargs):
    degrees = kwargs.get('degrees', None)
    adjacent = kwargs.get('adjacent', None)
    opposite = kwargs.get('opposite', None)

    if degrees:
        return math.tan(math.radians(degrees))
    elif opposite and adjacent:
        return(opposite / adjacent)


def find_missing_side(*args, **kwargs):
    sin = kwargs.get('sin', None)
    cos = kwargs.get('cos', None)
    tan = kwargs.get('tan', None)
    opposite = kwargs.get('opposite', None)
    hypotenuse = kwargs.get('hypotenuse', None)
    adjacent = kwargs.get('adjacent', None)

    if sin and hypotenuse:
        opposite = sin * hypotenuse
        return {'opposite': opposite}
    elif sin and opposite:
        hypotenuse = opposite / sin
        return {'hypotenuse': hypotenuse}
    elif cos and adjacent:
        hypotenuse = adjacent / cos
        return {'hypotenuse': hypotenuse}
    elif cos and hypotenuse:
        adjacent = cos * hypotenuse
        return {'adjacent': adjacent}
    elif tan and opposite:
        adjacent = opposite / tan
        return {'adjacent': adjacent}
    elif tan and adjacent:
        opposite = tan * adjacent
        return {'opposite': opposite}


def pythagorean(*args, **kwargs):
    opposite = kwargs.get('opposite', None)
    hypotenuse = kwargs.get('hypotenuse', None)
    adjacent = kwargs.get('adjacent', None)

    if hypotenuse and opposite:
        adjacent = math.sqrt(hypotenuse * hypotenuse - opposite * opposite)
    elif adjacent and opposite:
        hypotenuse = math.sqrt(adjacent * adjacent + opposite * opposite)
    elif hypotenuse and adjacent:
        opposite = math.sqrt(hypotenuse * hypotenuse - adjacent * adjacent)

    return {'hypotenuse': hypotenuse, 'adjacent': adjacent,
            'opposite': opposite}


if __name__ == "__main__":
    # Testing sin, cos, tan functions.
    print(sin(degrees=35))  # Answer is .57
    print(sin(opposite=2.8, hypotenuse=4.9))

    print(cos(degrees=35))  # Answer is .82
    print(cos(adjacent=4.0, hypotenuse=4.9))

    print(tan(degrees=35))  # Answer is .70
    print(tan(opposite=2.8, adjacent=4.0))

    # Testing find_missing_side function.
    print(find_missing_side(sin=sin(degrees=35), opposite=2.8))
    print(find_missing_side(cos=cos(degrees=35), hypotenuse=4.9))
    print(find_missing_side(tan=tan(degrees=35), adjacent=4.0))

    # Testing pythagorean function.
    hypotenuse = find_missing_side(sin=sin(degrees=35), opposite=2.8)
    print(pythagorean(opposite=2.8, hypotenuse=hypotenuse['hypotenuse']))
\$\endgroup\$
  • \$\begingroup\$ "without using the math module(except for 1 place)" why have you used math.sin, math.cos, math.tan, 3 math.radians and 3 math.sqrt. Isn't 1+1+1+3+3 = 9 not 1? \$\endgroup\$ – Peilonrayz Jul 27 at 18:56
  • \$\begingroup\$ A more accurate statement would be, I tried to write trig out in long form. Longer form would be to write out those equations by hand. Thank you for the inspiration to take this code farther. What are your thoughts other than my usage of math module? \$\endgroup\$ – Arlion Jul 27 at 19:05
5
\$\begingroup\$
  • All of your arguments accept *args but don't use it. If someone calls your functions like sin(some_angle), it's going to return None which is confusing. You can simply leave it out, **kwargs still works.

  • This is more a situation that calls for keyword-only arguments, because each function only has a small fixed number of possible arguments. This leads to better errors if, say, you misspell one of the argument names in a call. If you want to use keyword-only arguments without *args, you can add a single *, before the arguments, like this:

    def sin(*, degrees=None, opposite=None, hypotenuse=None):
        if degrees:
           return math.sin(math.radians(degrees))
        elif opposite and hypotenuse:
           return opposite / hypotenuse
    
  • Testing for possible arguments should be done with if x is not None instead of if x: getting sin(degrees=0) is totally valid but will return None if you test for degrees instead of degrees is not None!

  • You're sort of overloading each function based on arguments passed. This makes the functions error-prone to use and possibly confusing. You can call sin(degrees=60, opposite=1, hypotenuse=1) and your program won't complain. You can call sin() and get None.

    A possible solution would be to split the first four functions out like this:

    def sin_from_degrees(degrees)
    def sin_from_edges(*, opposite, hypotenuse)
    def cos_from_degrees(degrees)
    def cos_from_edges(*, adjacent, hypotenuse)
    def tan_from_degrees(degrees)
    def tan_from_edges(*, opposite, adjacent)
    def opposite_from_sin(*, sin, hypotenuse)
    def opposite_from_tan(*, tan, adjacent)
    def adjacent_from_cos(*, cos, hypotenuse)
    def adjacent_from_tan(*, tan, opposite)
    def hypotenuse_from_sin(*, sin, opposite)
    def hypotenuse_from_cos(*, cos, adjacent)
    

    Note how none of the arguments are optional any more, and if you try to use a function the wrong way, Python will give you a nice error message explaining exactly what went wrong. I also made degrees a regular argument, because I felt that sin_from_degrees(degrees=degrees) would be a bit redundant and there's no confusion about which argument means what if there's only one, although I can understand wanting the API to be more consistent.

  • Your last two function is interesting in that it is overloaded to an extent, but not as easy to extricate as the other functions. I would use a namedtuple instead of a dictionary, and definitely signal if pythagorean is called with the wrong arguments.

    from typing import NamedTuple
    
    class Triangle(NamedTuple):
        adjacent: float
        opposite: float
        hypotenuse: float
    
    def pythagorean(*, adjacent=None, opposite=None, hypotenuse=None):
        if hypotenuse is not None and opposite is not None and adjacent is None:
            adjacent = math.sqrt(hypotenuse * hypotenuse - opposite * opposite)
        elif adjacent is not None and opposite is not None and hypotenuse is None:
            hypotenuse = math.sqrt(adjacent * adjacent + opposite * opposite)
        elif hypotenuse is not None and adjacent is not None and opposite is None:
            opposite = math.sqrt(hypotenuse * hypotenuse - adjacent * adjacent)
        else:
            raise TypeError("pythogorean() expects exactly two arguments from 'adjacent', 'opposite', and 'hypotenuse'")
        return Triangle(adjacent, opposite, hypotenuse)
    

    There is still a bit of duplication (if a is not None and b is not None and c is None: c = math.sqrt(a * a ± b * b) occurs three times) but I'm not sure at the moment how to cleanly refactor that, especially due to the sign change when calculating the hypotenuse. You could make it a bit shorter by using comparator chaining (a is not None is not b) but honestly I think that would be hurt readability and maintainability more than it would help.

Maybe I've gone a bit overboard with suggesting changing things you didn't ask about, but I hope that at least answers your questions!

| improve this answer | |
\$\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.