Search code examples
design-patternsrefactoringdry

How can I not repeat writing similar code?


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

Solution

  • 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.

    How I got here

    Since you're asking for an approach, I'll try to describe how I came to this solution:

    1. Simplify the code

      • The outer while loop only ever ran once so it can be removed. This is key because it eliminates one use of the loop condition variables.
      • Then we see that each remaining 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!
      • Now we see that we need to change the order of operations. We need to save the value of 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.
    2. Notice the similarities in the code

      • Notice that in the two loops we shift the index but in opposite directions. We can add a shift value parameter and write the code such that the direction of the shift changes even with the same value.
      • Notice the direction of the comparison is different for the left and right loops in the upper and lower tangent calculation. To deal with this we can write a function that compares 2 things and returns -1 if the first is greater, 1 if the second is greater, and 0 if they are the same.
      • Now that we have the shift value that will have different signs based on whether we are looking for upper or lower, we can use that to flip the direction of the 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!