patternpythonMinor
Python Trig Calculator
Viewed 0 times
trigcalculatorpython
Problem
It's big, it's ugly, but it should work. My question is: how could I have implemented the "typeselection" function so it is less repetitive? Any tips on other improvements in coding style are welcomed as well.
```
import math
import turtle
class Triang:
#MATHS FUNCTIONS; THESE GET CALLED WHEN PROGRAM HAS DECIDED ON TRIANGLE TYPE
def dass(self, arg, arg1, arg2, ans, args, args1, args2):
arg2=math.radians(arg2)
arg1=math.radians(arg1)
print (ans, "= ( ", args, "* sin (", args1, ") / ( sin (", args2, ") )")
a=(arg*math.sin(arg1))/math.sin(arg2)
print (ans, "=", a)
return a
def sss1(self, arg, arg1, arg2, ans, args, args1, args2):
print (ans, "= acos ( (", args, "^ 2 +", args1, "^ 2 -", args2, "^ 2 ) / ( 2 ", args, "", args1, ") )")
a=math.degrees(math.acos((arg2+arg12-arg2**2)/(2argarg1)))
print (ans, "=", a)
return a
def sas(self, arg, arg1, arg2, ans, args, args1, args2):
arg2=math.radians(arg2)
print (ans, "^ 2 =", args, "^ 2 +", args1, "^ 2 - 2 ", args1, " cos ( ", args2, ")")
a=(arg2+arg12-2arg1arg1*math.cos(arg2))
print (ans, "^ 2 =", a)
if a = 3 :
self.sides()
else :
self.angles()
def obtusecol(self):
self.oa = self.a if self.a else False
self.ob = self.b if self.b else False
self.oc = self.c if self.c else False
self.oab = self.ab if self.ab else False
self.oac = self.ac if self.ac else False
self.obc = self.bc if self.bc else False
def collector(self, arg, arg1, arg2="A"):
print (" ")
print ("Enter the value for,", arg1, arg)
if arg1=="side":
print ("Some people would call that side", arg2)
a=input(">>")
try:
a=float(a)
except ValueError:
if a=="x" or a=="X":
a=False
else:
```
import math
import turtle
class Triang:
#MATHS FUNCTIONS; THESE GET CALLED WHEN PROGRAM HAS DECIDED ON TRIANGLE TYPE
def dass(self, arg, arg1, arg2, ans, args, args1, args2):
arg2=math.radians(arg2)
arg1=math.radians(arg1)
print (ans, "= ( ", args, "* sin (", args1, ") / ( sin (", args2, ") )")
a=(arg*math.sin(arg1))/math.sin(arg2)
print (ans, "=", a)
return a
def sss1(self, arg, arg1, arg2, ans, args, args1, args2):
print (ans, "= acos ( (", args, "^ 2 +", args1, "^ 2 -", args2, "^ 2 ) / ( 2 ", args, "", args1, ") )")
a=math.degrees(math.acos((arg2+arg12-arg2**2)/(2argarg1)))
print (ans, "=", a)
return a
def sas(self, arg, arg1, arg2, ans, args, args1, args2):
arg2=math.radians(arg2)
print (ans, "^ 2 =", args, "^ 2 +", args1, "^ 2 - 2 ", args1, " cos ( ", args2, ")")
a=(arg2+arg12-2arg1arg1*math.cos(arg2))
print (ans, "^ 2 =", a)
if a = 3 :
self.sides()
else :
self.angles()
def obtusecol(self):
self.oa = self.a if self.a else False
self.ob = self.b if self.b else False
self.oc = self.c if self.c else False
self.oab = self.ab if self.ab else False
self.oac = self.ac if self.ac else False
self.obc = self.bc if self.bc else False
def collector(self, arg, arg1, arg2="A"):
print (" ")
print ("Enter the value for,", arg1, arg)
if arg1=="side":
print ("Some people would call that side", arg2)
a=input(">>")
try:
a=float(a)
except ValueError:
if a=="x" or a=="X":
a=False
else:
Solution
Just general style
What the other comment said is true, the style can be improved here.
the mathematical stuff, there are often already established names
for the parameters to functions which you can use.)
parameters for it. (The code calling this can't observe the difference
between whether the "ab" or "ac" string is passed -- this makes little
sense for that caller.)
the class, so that you can focus on either math or UI, and have a smaller
piece of code to look at for each.
Taking apart these cases
In the
For example, take this code. In this case, we already know that the angles a, b, c are given.
In cases 4, 5 and 6, you already have all angles and two lengths of the triangle given. These may conflict already with each other. You may collapse this with one of the other cases above if you want to ignore that, or you check for the conflict. So we are getting a bit tangled up in all the different cases here... :)
Another approach: Loop until you can't apply any of the rules any more
Look at what you need for each individual calculation:
This just needs ab, b and c to work, and assumes that ac is not known.
But the code is also guarded by other assumptions right now. You could try to only guard it by the things you really need (ab, b and c are there, ac is not), and try out all the formulas until you can't do any changes any more. :)
Then each way to calculate ac would only need to be mentioned once. You can also take the whole part within
Constraint propagation, if you want to get fancy
There is also a way to calculate this "lazily" without a giant
http://mitpress.mit.edu/sicp/full-text/book/book-Z-H-22.html#%_sec_3.3.5
This is basically a graph of present or non-present values which are connected to each other through mathematical transformations. For instance, you might have the graph nodes X, Y and Z with the connecting transformation that says "X * Y = Z". Now the transformation is represented as an object and gets notified whenever one of the values is suddenly available. Once there are two values available, the transformation will calculate the third value and set it. For instance, when we know that Y=3 and Z=15, it will calculate and set X to 5 (but it will do division or multiplication, depending on which inputs it gets! :)). Then when it sets X to 5, there may be other transformations in the network that now get triggered because they were waiting for a value for X. Another nice upside to this is that the network can detect when there are conflicting inputs in the system.
What the other comment said is true, the style can be improved here.
- meaningful names (Calling arguments
argis not so helpful :) For
the mathematical stuff, there are often already established names
for the parameters to functions which you can use.)
- logging is ok with print, but I would not introduce additional
parameters for it. (The code calling this can't observe the difference
between whether the "ab" or "ac" string is passed -- this makes little
sense for that caller.)
- the user interaction should be separated from the math, ideally outside
the class, so that you can focus on either math or UI, and have a smaller
piece of code to look at for each.
Taking apart these cases
In the
typeselection method, you can still simplify many of your cases by thinking about the math.For example, take this code. In this case, we already know that the angles a, b, c are given.
if self.a and self.b and self.c:
if self.ab and not self.bc and not self.ac: # Case 1
self.deter()
self.ac=self.dass(self.ab, self.b, self.c, "ac", "ab", "b", "c")
self.bc=self.dass(self.ac, self.a, self.b, "bc", "ac", "a", "b")
elif self.bc and not self.ab and not self.ac: # Case 2
self.deter()
self.ab=self.dass(self.bc, self.c, self.a, "ab", "bc", "c", "a")
self.ac=self.dass(self.bc, self.b, self.a, "ac", "bc", "b", "a")
elif self.ac and not self.bc and not self.ab: # Case 3
self.deter()
self.ab=self.dass(self.ac, self.c, self.b, "ab", "ac", "c", "b")
self.bc=self.dass(self.ac, self.a, self.b, "bc", "ac", "a", "b")
elif self.ab and self.bc and not self.ac: # Case 4
self.deter()
self.ac=self.dass(self.bc, self.b, self.a, "ac", "bc", "b", "a")
elif self.ab and self.ac and not self.bc: # Case 5
self.deter()
self.bc=self.dass(self.ab, self.a, self.c, "bc", "ab", "a", "c")
elif self.bc and self.ac and not self.ab: # Case 6
self.deter()
self.ab=self.dass(self.ac, self.c, self.b, "ab", "ac", "c", "b")In cases 4, 5 and 6, you already have all angles and two lengths of the triangle given. These may conflict already with each other. You may collapse this with one of the other cases above if you want to ignore that, or you check for the conflict. So we are getting a bit tangled up in all the different cases here... :)
Another approach: Loop until you can't apply any of the rules any more
Look at what you need for each individual calculation:
self.ac=self.dass(self.ab, self.b, self.c, "ac", "ab", "b", "c")This just needs ab, b and c to work, and assumes that ac is not known.
But the code is also guarded by other assumptions right now. You could try to only guard it by the things you really need (ab, b and c are there, ac is not), and try out all the formulas until you can't do any changes any more. :)
while True:
if not self.ac:
if self.ab and self.b and self.c:
self.ac = self.dass(self.ab, self.b, self.c)
continue # Next loop iteration
elif self.bc and self.b and self.a:
self.ac = self.dass(self.bc, self.b, self.a)
continue # Next loop iteration
# etc.
# etc.; make sure to get all cases
break # Break out of loopThen each way to calculate ac would only need to be mentioned once. You can also take the whole part within
if not self.ac and extract it into a single method where the names of the corner points are exchanged. (Replace self.xxx with the parameters of that function.)Constraint propagation, if you want to get fancy
There is also a way to calculate this "lazily" without a giant
while True loop, but that's much more difficult. I wouldn't recommend it to you now, but I'll still mention it because it's fancy, and you may want to revisit in the future. It takes the "keep track of constraints between things" problem to another level of abstraction. :)http://mitpress.mit.edu/sicp/full-text/book/book-Z-H-22.html#%_sec_3.3.5
This is basically a graph of present or non-present values which are connected to each other through mathematical transformations. For instance, you might have the graph nodes X, Y and Z with the connecting transformation that says "X * Y = Z". Now the transformation is represented as an object and gets notified whenever one of the values is suddenly available. Once there are two values available, the transformation will calculate the third value and set it. For instance, when we know that Y=3 and Z=15, it will calculate and set X to 5 (but it will do division or multiplication, depending on which inputs it gets! :)). Then when it sets X to 5, there may be other transformations in the network that now get triggered because they were waiting for a value for X. Another nice upside to this is that the network can detect when there are conflicting inputs in the system.
Code Snippets
if self.a and self.b and self.c:
if self.ab and not self.bc and not self.ac: # Case 1
self.deter()
self.ac=self.dass(self.ab, self.b, self.c, "ac", "ab", "b", "c")
self.bc=self.dass(self.ac, self.a, self.b, "bc", "ac", "a", "b")
elif self.bc and not self.ab and not self.ac: # Case 2
self.deter()
self.ab=self.dass(self.bc, self.c, self.a, "ab", "bc", "c", "a")
self.ac=self.dass(self.bc, self.b, self.a, "ac", "bc", "b", "a")
elif self.ac and not self.bc and not self.ab: # Case 3
self.deter()
self.ab=self.dass(self.ac, self.c, self.b, "ab", "ac", "c", "b")
self.bc=self.dass(self.ac, self.a, self.b, "bc", "ac", "a", "b")
elif self.ab and self.bc and not self.ac: # Case 4
self.deter()
self.ac=self.dass(self.bc, self.b, self.a, "ac", "bc", "b", "a")
elif self.ab and self.ac and not self.bc: # Case 5
self.deter()
self.bc=self.dass(self.ab, self.a, self.c, "bc", "ab", "a", "c")
elif self.bc and self.ac and not self.ab: # Case 6
self.deter()
self.ab=self.dass(self.ac, self.c, self.b, "ab", "ac", "c", "b")self.ac=self.dass(self.ab, self.b, self.c, "ac", "ab", "b", "c")while True:
if not self.ac:
if self.ab and self.b and self.c:
self.ac = self.dass(self.ab, self.b, self.c)
continue # Next loop iteration
elif self.bc and self.b and self.a:
self.ac = self.dass(self.bc, self.b, self.a)
continue # Next loop iteration
# etc.
# etc.; make sure to get all cases
break # Break out of loopContext
StackExchange Code Review Q#89958, answer score: 7
Revisions (0)
No revisions yet.