I have two very similar functions that is almost a duplicate of each other except that the conditions are opposite to each other. I want to follow the DRY pattern, but I have read somewhere before that passing a boolean into a function to change behavior is a bad practice(since its difficult to maintain). What is my best option here for me to write clean and non repetitive code? Thanks
def find_upper_tangent(left, right):
l_idx = get_rightmost(left)
r_idx = get_leftmost(right)
upper_tangent_left = False
upper_tangent_right = False
prev_slope = get_slope(left[l_idx], right[r_idx])
while not upper_tangent_left or not upper_tangent_right:
while not upper_tangent_left:
# move counter clockwise
next_slope = get_slope(left[(l_idx - 1) % len(left)], right[r_idx])
if next_slope < prev_slope:
l_idx = (l_idx - 1) % len(left)
prev_slope = next_slope
upper_tangent_right = False
else:
upper_tangent_left = True
while not upper_tangent_right:
# move clockwise
next_slope = get_slope(left[l_idx], right[(r_idx + 1) % len(right)])
if next_slope > prev_slope:
r_idx = (r_idx + 1) % len(right)
prev_slope = next_slope
upper_tangent_left = False
else:
upper_tangent_right = True
return l_idx, r_idx
def find_lower_tangent(left, right):
l_idx = get_rightmost(left)
r_idx = get_leftmost(right)
lower_tangent_left = False
lower_tangent_right = False
prev_slope = get_slope(left[l_idx], right[r_idx])
while not lower_tangent_left or not lower_tangent_right:
while not lower_tangent_left:
# move clockwise
next_slope = get_slope(left[(l_idx + 1) % len(left)], right[r_idx])
if next_slope > prev_slope:
l_idx = (l_idx + 1) % len(left)
prev_slope = next_slope
lower_tangent_right = False
else:
lower_tangent_left = True
while not lower_tangent_right:
# move counter clockwise
next_slope = get_slope(left[l_idx], right[(r_idx - 1) % len(right)])
if next_slope < prev_slope:
r_idx = (r_idx - 1) % len(right)
prev_slope = next_slope
lower_tangent_left = False
else:
lower_tangent_right = True
return l_idx, r_idx
I think something along the lines of this should do the same calculation:
def find_upper_tangent(left, right):
return find_tangent(left, right, -1)
def find_lower_tangent(left, right):
return find_tangent(left, right, 1)
def find_tangent(left, right, shift_val):
l_idx = get_rightmost(left)
r_idx = get_leftmost(right)
prev_slope = get_slope(left[l_idx], right[r_idx])
next_slope = prev_slope
while compare(next_slope, prev_slope) == (shift_val / shift_val):
# move counter clockwise
prev_slope = next_slope
l_idx = (l_idx + shift_val) % len(left)
next_slope = get_slope(left[l_idx], right[r_idx])
while compare(next_slope, prev_slope) == (shift_val / -shift_val)):
# move clockwise
prev_slope = next_slope
r_idx = (r_idx - shift_val) % len(right)
next_slope = get_slope(left[l_idx], right[r_idx])
return l_idx, r_idx
def compare(v1, v2):
return -1 if v1 > v2 else 0 if v1 == v2 else 1
IMO this is better than passing a flag because you're passing a value that has meaning and is used in the calculation.
Since you're asking for an approach, I'll try to describe how I came to this solution:
Simplify the code
while
loop only ever ran once so it can be removed. This is key because it eliminates one use of the loop condition variables.while
loop has an else
clause that only sets the loop condition variable. Instead we can use the simply use the if
conditional as the loop condition. This eliminates the need the booleans variables and reduces the number of code paths - both great goals for simplifying code!next_slope
before computing the new value. We are also doing the calculation for the new index twice. Once to get the slope and once to save the value. Instead, we can compute the new index and then use that in the slope calculation.Notice the similarities in the code
while
condition.I haven't verified that this solution works - if you want to post some test data and expected result, that would be great!