1

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.

4

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.

  • There is a violation of DRY: the docstring which describes the API. If the behaviour of call_the_same_method is changed or extended, the repeated comments have to be adapted in multiple places, and no place must be overlooked or forgotten. It would be nice to have a way to reuse docstrings, or parts of them, or set links from one docstring to another which can be evaluated by some doc generators, but I don't know if there are environments available which support that, or if there is a solution for specific languages like Python. – Doc Brown Nov 24 '18 at 9:04
  • @DocBrown: Yeah docstrings are repeated in the example, but I assume that would not happen in the real world. If the methods actually do something different why would they have the same documentation? Also I disagree the docstrings should specify implementation details like which underlying methods are called. – JacquesB Nov 24 '18 at 9:12
  • "I assume that would not happen in the real world" - but you read the very first sentence in the question, where the OP descreibed where they found this kind of repetition? And as I wrote, this is not about comments describing the implementation details. For example, when the error handling inside call_the_same_method is changed, and that method now can throw a new kind of exception, all the docstrings for those get_some_stuff_type have to be extended, one by one. – Doc Brown Nov 24 '18 at 9:24
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.

Your Answer

By clicking "Post Your Answer", you acknowledge that you have read our updated terms of service, privacy policy and cookie policy, and that your continued use of the website is subject to these policies.

Not the answer you're looking for? Browse other questions tagged or ask your own question.