How can I resolve this **kwargs antipattern?

Question

I am a hobbyist programmer, working on a much more complex Python project than I've attempted before, which is in the form of a Python library.

I find that I'm often passing **kwargs around (as described in this question), but I'm also realising it's bad for readability and maintainability. The answer to the previous question suggests that the code may need to be refactored, but I am unsure how to do so while still achieving what I was trying to achieve in the first place.

Here's an example of what I find myself doing. My application is built around Shape objects that can be displayed on screen. Displaying them is quite a complex task, so I have a DisplayManager class that looks like this:

# displayManager.py

class DisplayManager:
    def __init__(self):
        # set up display context, open a window, etc.
    def display_shape(self, shape, color, opacity=1):
        # query the Shape object for the data needed
        # to display it, then process that data and
        # show it on screen

It makes sense to put the display method here rather than in the Shape class, because the code and libraries used for displaying are quite distinct from the rest of the application's logic, so I wanted to keep it separate.

But most of the time the user shouldn't have to worry about the displayManager class. So I end up having a global displayManager object and putting a convenience display method in Shape, like this:

# shape.py
import display

class Shape:
    # ...
    def display(self, color, opacity=1):
        display.globalDisplayManager.display_shape(self, opacity)

The problem is that now I have the arguments color and opacity in two different places in the code, in separate files, which violates DRY. It's quite likely that these arguments or their default values will change in the future, and I would have to remember to update them in both places. So instead, I end up doing this:

# shape.py
import display

class Shape:
    # ...
    def display(self, *args, **kwargs):
        display.globalDisplayManager.display_shape(self, *args, **kwargs)

Now the arguments are specified in only one place, and if I decide to change them or add new ones I only have to do it in one place, in display.py where the display routines are implemented.

The problem is that it's not very good from the user's point of view. Looking at Shape.display doesn't actually tell you its arguments, and to find that out you have to delve into the DisplayManager class, which I didn't want the user to worry about.

Of course this can be resolved by describing the arguments in the docstring for Shape.display, but then I'd have an even worse violation of DRY, since the documentation would be in a different place from the implementation and it would be easy to forget to keep in them in sync.

So after that long description, my question is how (or whether) I can achieve my goals without this big disadvantage? That is, how can I keep the display implementation separate from the Shape implementation, in such a way that (i) I don't have to remember to update multiple files if I change the API, and (ii) the user doesn't have to dig into the implementation to find out how to use it? More broadly, if I find myself passing *args and **kwargs around like this, what should I be considering instead?

_{(Note: a possible response to this question is that I shouldn't be changing the API all the time anyway. My reply is that at this stage the only user is me. The API will all be locked down before I release, if I ever do release, but for now it's important to keep it flexible so I can figure out what works.)}

Answer 1

Dividing responsibilities between classes is tricky since there's no right answer and it'll take future usage patterns to find out what makes for better answers. Anyway it's inevitable that the classes will call on each other, each handling its own responsibility and passing the buck -- and several argument values.

Yes, adding features like texture and line style will require adding arguments to some methods, or maybe just adding methods like set_texture(). Similarly, color could be an argument or a property of the shape or of the canvas.

The least DRY part of your example is the default argument values. Well, the default opacity 1 is unlikely to change, but you could define a constant for each default value and use that in all relevant method definitions in all the classes.

Interfaces will evolve. In fact, the great thing about interfaces is they're easy to evolve -- although if they have unknown clients out in the world, one should stick with compatible changes. Meanwhile, a refactoring editor such as PyCharm makes it easier to make consistent method signature changes across multiple files including their docstrings (although this feature doesn't work as smoothly in Python as in Java, for want of the type declarations).

You are right that *args, **kwargs is unhelpful to clients. It's also unhelpful to code inspection tools and documentation tools, such as asking PyCharm to display a method's parameters when you're coding a call to it. I'd prioritize these issues over the DRY concern.

There are different kinds of "DRY violations," with different severities. I suggest not worrying overly much about that design guideline.

If your project is a graphics package, i.e. that's not just your example for this question, then a good tip is to look at various graphics APIs to see how they addressed the same design question.

Answer 2

Semantic Fix

Why is that the users should not be aware of this display?

I feel that this is the core issue. What precisely is a Display, and will there be nuanced versions of it? Say a display for the screen, and a display for a printer? In such a case only the using code will known which to select.

Even if there is only one implementation, you have already pointed out that Shape has no coupling to Display, so you are keeping them separate. Yet the immediate solution is to couple the shape to the display? This will make it hard to make changes later.

The best option would be to keep that separation of concern. If you must couple them somehow, consider a Scene object which holds a collection of Shapes, and a reference to a Display. It would then be ideally suited to help optimise draws, and perhaps identifying the exact shape the mouse is over. This will simplify the management of many drawing related concerns.

Syntatic Fix

You will need define another value as representing the default, and mapping that to the real default later:

class Shape:
    # ...
    def display(self, color, opacity="default"):
        display.globalDisplayManager.display_shape(self, color, opacity)


class SomeDisplayManager:
    # ...
    def display(self, color, opacity="default"):
        # provide the real default...
        if opacity == "default":
            opacity = 1         
        #do stuff

Answer 3

**kwargs when used this way has a few downsides - first, you're puncturing through encapsulation of your components, so everything knows about everything else, and second, by having a call stack full of undefined variables, you open yourself up to some very confusing bugs where one call uses a variable that you meant to pass further down the stack. Last but not least, it's a nightmare to document it, because everything in your call stack has a potential argument for everything else.

For what you're doing, there are a couple good ways to fix this:

• Use object state. Add color and opacity as properties of Shape, and reference them when needed. The code defining the shape now sets those fields before calling display. This strengthens the encapsulation of the program, as your manager call doesn't need to be told what the color and opacity are

• Use an object named something like DisplayParameters and pass it into the shape's call to display. This will make clear to the caller what information they passed in is going to go where, and gives a clear hook for documentation of what you're passing in.

• Consider keeping these two parts decoupled. It looks like you've got a global that you're using so your convenience functions can get a reference to a DisplayManager. Without a good way to swap out or mock that DisplayManager, you're going to have a hard time testing those functions, and you might not be saving yourself that much client code for the trouble that this coupling gets you into.