DRY or not to DRY in SDK

Question

I'm making contribution to an opensource project by one of the leading hypervisor providers and I have noticed that there is awfully lot of repetition.

Below is what I mean by it, imagine that they do this multiple times in the same class and then across other classes.

def get_some_stuff_type_A(self, name):
    """The same Text
    """
    result = call_the_same_method(name=name, type=A)
    return result

def get_some_stuff_type_B(self, name):
    """The same Text
    """
    result = call_the_same_method(name=name, type=B)
    return result

def get_some_stuff_type_C(self, name):
    """The same Text
    """
    result = call_the_same_method(name=name, type=C)
    return result

Should this be squashed into one method with just another param of type to make it DRY?

Or is this permissible since this is an SDK and this provides better visibility down the line when working with it? Although still very ugly repetition.

Answer 1

This is not a violation of DRY. It is a facade which hides the implementation detail that get_some_stuff_type_A and get_some_stuff_type_B uses the same underlying method. This is often good API design, since the implementation could change in the future to use different methods without affecting the interface. (Whether it is good design in this particular case is hard to say since the example is so abstract, but the principle is fine.)

To take a more concrete example:

def suspend_user():
     set_user_state('suspended')
def delete_user():
     set_user_state('deleted')
private def set_user_state(state)
     set_state_enum_in_database(state)

You could just expose set_user_state(state) directly and let the caller provide the state value, right? But what if you want to change the implementation to actually delete the record from the database? Oops, you can't do that now. Or what if it delete_user should take a flag which indicates if the user is permanently deleted? Too late. By exposing the state enum in the API you have coupled the API to a particular implementation.

On the other hand there may be contexts where the second approach is better. Lets say there is a text rendering routine:

def print_letter_A():
    print_character('A')
def print_letter_B():
    print_character('B')

In this case there is no value to have individual methods for each letter, so you could simplify the API to just expose print_character(char).

So the actual answer to your question depends on what "stuff" is.

DRY is a good principle in general, but is sometimes misunderstood. It does not say you should compress the code to contain the lowest number of tokens. It says the same information should only be represented once. But in the case of an API facade you have two different kind of information - how the API interface looks and how it is actually implemented. These are supposed to be different and be able to change independently, otherwise you have a tight coupling.

Answer 2

I would vote for squashing, as long as the code indeed does the same thing.

To my mind, code should express the intent. If it's the same intent, it should run the same code, not multiple copies of it.

I'd also try to think about the interface. Its ergonomics do not look all that great, with similar repetitive methods. OTOH if users mostly just call one method with an expressive name, it may be a good approach.