0

I'm in the process of combing through and simplifying a codebase for a python project. The nature of the selenium-wrapping library that I'm using called Splinter is that I find myself writing a lot of code with minor differences, maybe in the element I'm searching for, or a tiny change in selection logic, different parameters, etc.

Currently, I might have two separate functions with 90% of the code copy and pasted between them. My two ideas for condensing this and being consistent are as follows:

1) Create three functions: A, B, and C. Functions A and B would be called directly and have single parameters. These functions then call function C with both the parameter they were given, and then the correct 'identifying' parameter, to change the way that function C works. Function C would never be called directly.

def A(x):
    return C(x, 0)

def B(y):
    return C(y, 1)

def C(a, b):
    if b:
        # Logic for B
    else:
        # Logic for A

2) Create one function, and have it take two parameters. First parameter is what you would otherwise pass into A or B, and the second parameter is the 'identifier' portion.

def D(x,i):
    if i == 'case 1':
    elif i == 'case 2':
    else:

The 'identifier' part is what I don't like. Is it a smell to have a function depend on the caller using specific keywords, or values, in a parameter?

Option 1 produces two more functions to take care of, but option 2 causes a user to know these special 'ID' values in order for the function call to work correctly.

Which of these two patterns would be a better solution in this case?

chepner
  • 497,756
  • 71
  • 530
  • 681
Garrett
  • 1
  • 1
  • Your two examples are the same thing, only one uses an int (faster, less identifying), the other a string (slower comparison). Using an enum would be better. However, I do not know why `C()` is one function, not two. (And also why `A()` and `B()` are two functions...) – Draco18s no longer trusts SE Feb 02 '16 at 20:04
  • @Draco18s The actual flag used to identify the cases can be changed, I was just being explicit in the example. I don't quite know what you mean with the questions at the end of your comment. A and B are currently two functions because one might access a different element on the page, in a different order than the other. Like I said in the OP, it's these repetitive minor differences. – Garrett Feb 02 '16 at 21:15
  • My point was that "repetitive minor differences" mean that a large portion of the code is the same. *That code* is functionally identical and could be a function on its own. Perhaps not, but that's what it sounds like. – Draco18s no longer trusts SE Feb 02 '16 at 21:26

2 Answers2

1

This sounds like a good reason to use a decorator function. The wrapper function does the duplicate code while the innermost function is defined as the separate code. For example see the answers for How to make a chain of function decorators?

Thus, you would make the common code A and two functions B and C

def A():

@A
def B():

@A
def C():

For example Understanding Python Decorators in 12 Easy Steps!

Community
  • 1
  • 1
sabbahillel
  • 4,357
  • 1
  • 19
  • 36
1

Out of the two options presented, the first sounds like a better approach.

This is because the second option unnecessarily reveals an implementation detail which all callers must know about and depend on.

When implementing the first option, you can make the shared function, C(), private in the class or module. In python, this is often done by naming convention: prepend the function name with a single underscore for module privates and prepend the function name with double underscores for class privates.

Gary McLean Hall
  • 984
  • 7
  • 16