Today one of my coworkers came and got me so that I could explain some weird Python code they’d found. It dealt with cmake, but since it was internal code, I won’t be able to show it here. Instead, I wrote up something that has the same issues so you can see what I consider bad, or at the very least, very obtuse code:
class Config:
def Run(self):
print('Program Usage: blah blah blah')
print(self.product)
self.asset_tag = 'test'
print(self.asset_tag)
total = self.sub_total_a + self.sub_total_b
When I first saw this, I was struck by how it was using attributes that weren’t initialized and I wondered how this could work. Then I realized they must be doing some kind of monkey patching. Monkey patching is where you write code to dynamically modify a class or module at run time. So I looked farther down the script and found some code that created an instance of the class and did something like this:
def test_config():
cfg = Config()
cfg.product = 'laptop'
cfg.asset_tag = '12345ABCD'
cfg.sub_total_a = 10.23
cfg.sub_total_b = 112.63
cfg.Run()
if __name__ == '__main__':
test_config()
So basically whenever you create an instance of the class, you need to patch it so that the attributes exist before you call the Run method.
While I think it’s pretty cool that Python can do this sort of thing, it is extremely confusing to someone who is not familiar with Python, especially when the code was as poorly documented as this was. As you can see, there are no comments or docstrings, so it took a bit of digging to figure out what was going on. Fortunately, all the code was in the same file. Otherwise this could have gotten really tricky to figure out.
I personally thought this was a good example of bad coding. If I had written it, I would have created an __init__ method and initialized all those attributes there. Then there would have been no confusion about how the class worked. I am also a big believer in writing good docstrings and useful comments.
Anyway, I hope you found this interesting. I also thought my readers should be aware of some of the oddball pieces of code you’ll see in the wild.
Maybe it is only me that thinks about it the wrong way, but I would have said they are instance variables (or attributes) rather than class variables. Class variables to me is where one did Config.product = ‘laptop’. In other words, class variables to me is setting it on the type. I still agree though that an initialiser method should have been used with parameters being passed when the instance was created. I am not sure I would call what they were doing monkey patching though.
It’s called no-init and attribute-defined-outside-init, and Pylint catches it, like the numerous other problems of this code. It’s also called “you really need to turn up that QA/CI knob”. 🙂
It’s not monkey patching.
Why even use class? If you don’t need to keep state, a single function with four arguments should be enough.
You’re right. They are attributes. I was tired last night and called them the wrong thing. While I don’t consider the class to be an example of monkey patching, since the function modifies the object at runtime, that seemed like an instance of monkey patching to me
I agree that Pylint would have caught it. Sadly our organization doesn’t use that handy tool right now. I felt that the function that created an instance of the class was doing the monkey patching since it’s modifying the object at run time.
We have a pretty good QA team for the C++ side of things, but the Python side still needs a lot of work
Who knows!? I didn’t write this and neither did my colleague. I just wanted to share this code so my readers would know what kinds of things you see out there.
__init__ actually acts as documentation of a class.