python circular imports once again (aka what's wrong with this design)
Circular imports are not inherently a bad thing. It's natural for the team
code to rely on user
whilst the user
does something with team
.
The worse practice here is from module import member
. The team
module is trying to get the user
class at import-time, and the user
module is trying to get the team
class. But the team
class doesn't exist yet because you're still at the first line of team.py
when user.py
is run.
Instead, import only modules. This results in clearer namespacing, makes later monkey-patching possible, and solves the import problem. Because you're only importing the module at import-time, you don't care than the class inside it isn't defined yet. By the time you get around to using the class, it will be.
So, test/users.py:
import test.teams
class User:
def setTeam(self, t):
if isinstance(t, test.teams.Team):
self.team = t
test/teams.py:
import test.users
class Team:
def setLeader(self, u):
if isinstance(u, test.users.User):
self.leader = u
from test import teams
and then teams.Team
is also OK, if you want to write test
less. That's still importing a module, not a module member.
Also, if Team
and User
are relatively simple, put them in the same module. You don't need to follow the Java one-class-per-file idiom. The isinstance
testing and set
methods also scream unpythonic-Java-wart to me; depending on what you're doing you may very well be better off using a plain, non-type-checked @property
.
i. To make it work, you can use a deferred import. One way would be to leave user.py alone and change team.py to:
class team:
def setLeader(self, u):
from test.user import user
if issubclass(u, user.__class__):
self.leader = u
iii. For an alternative, why not put the team and user classes in the same file?
Bad practice/smelly are the following things:
- Probaly unnecessary type checking (see also here). Just use the objects you get as it were a user/team and raise an exception (or in most cases, one is raised without needing additional code) when it breaks. Leave this away, and you circular imports go away (at least for now). As long as the objects you get behave like a user / a team, they could be anything. (Duck Typing)
- lower case classes (this is more or less a matter of taste, but the general accepted standard (PEP 8) does it differently
- setter where not necessary: you just could say:
my_team.leader=user_b
anduser_b.team=my_team
- problems with data consistency: what if
(my_team.leader.team!=my_team)
?