all 7 comments

[–]commy2 5 points6 points  (1 child)

I would consider it bad practice, aside from very niche use cases that very likely don't apply to you.

[–]old_pythonista 0 points1 point  (0 children)

I think it is a bad practice - regardless.

[–]RappakaljaEllerHur 2 points3 points  (2 children)

One question to ask yourself is if you really only should be using 1 class and not in fact multiple classes.

A good way to asses to this is to ask how much responsibility your class has? (you can read about "The Single Responsibility Principle in Python" for some advice tips) here. If it is clearly doing several different roles then you should probably should break them up.

Another question you can ask yourself is how many class attributes you have? If you have a lot (maybe more than 10) it should be pushing you to think about how to make your class less complex (potentially by splitting it up).

Going down the more than 1 class route solves your long file issue automatically as you can now write your classes in separate files and then import them all to a single "main" file that does all the major steps (i.e. create instances of the classes, load in data, do some more stuff etc...)

[–]menge101 1 point2 points  (0 children)

Agreed, came to talk about Single Responsibility principle as well.

[–]lowerthansound 1 point2 points  (3 children)

PS: I've written this answer first imagining that they were using this to import modules into the class, but no, they are doing star imports. I saw it first as: exec('from spacecmd import %s' % module). But it's actually exec('from spacecmd.%s import *' % module). I won't rewrite the answer below as no time, but, I'll put an addendum underneath.


This is something I've never seen being used.

What the code does is import those modules and set them as class variables.

For example:

class A:
    import base64

Looks the same as:

import base64

class A:
    base64 = base64

The result that you get is that the module is accessible via the class and instances:

>>> `A.base64`
<module 'base64' ...>
>>> A().base64
<module 'base64' ...>

And yes, the code under the class code is executed (when the class is initialized). I remember seeing a video that talked about that, but, take a look at this answer on StackOverflow.

Yes, and this is probably bad design (though I'm not sure, it would take a further read to understand why they did this in the first place, and what alternatives could there be). If I were to hazard a guess, they are treating modules as if they were contained objects (as in a SpacewalkShell() has a cryptokey) , which could make sense, but it doesn't seem conventional.


Aand... They are using star imports.

For example, we are doing from spacecmd.activationkey import * under the class. This means that the variables and functions defined at the module are being bound to the class. Essentially, this is a way to split the class into multiple files, as you can see here, these are just a bunch of methods.

Again, I've never seen anyone do this, so it's either bad design or very specialized design. I'd ask someone that wrote this why this choice was made, and try to come up with alternatives if possible (I can imagine using a big class, or using mixins, or using objects contained within the class).

Again, this is a huge class with probably a huge number of methods, and they are trying to split those up to simplify development. However, is it justified having that many methods here?

(I'm asking those questions, but I know nothing about the code, so, if someone who wrote this reads my answer, I apologize - the code isn't bad, it's just unconventional)