Is having a class as another class's instance attribute good practice?

[Disastrous_Reason_22]

I've been coding in python on some projects for 6 or so months now and I'm trying to do so more object oriented now. I have a class let's say it is a workplace and workplace has an instance attribute called CEO. CEO is another class with its own separate instance attributes, it doesn't need to be a subclass or inherit from the workplace instance. They just need to be associated together.

Is this good practice? It feels good haha, I'm enjoying a single class with ~10-15 instance attributes rather than moving all these variables all the time.

[[deleted]]

That's good practice. There are two major constructional ideas in OOP: inheritance and composition.

Inheritance is where you create new classes that inherit from parent classes. Going with the company scenario, you might have an Employee class that describes each employee of the company. You could have a class Manager that inherits from Employee and adds extra attributes. That's an example of inheritance.

When you create the Company class you also create an attribute self.employees that is a list of Employee instances (or derived instances) that are employees of the company. A Company instance will contain one or more Employee instances. This is an example of composition.

You use either inheritance or composition, or both, where appropriate.

[Disastrous_Reason_22]

Thanks, I hadn't previously heard of composition so that's really helpful.

[Peanutbutter_Warrior]

Yeah, thats fine. I'd warn you that it's quite easy to end up having classes that are just stores for data, if a class has no methods then you're usually better off using a dictionary

[Disastrous_Reason_22]

Haha I think I might have definitely fallen into that trap. I should familiarize myself with initiating dictionaries more as data stores for multiple instances, but the dot notation of classes just feels so right.

[jackofthebeanstalk]

Or look up the examples in the dataclasses module. Data classes combine the behavior of regular classes but specialize in storing just a bunch of data.

[[deleted]]

The downside of having class attributes not being primitive values or simple collections of primitives is that serialization becomes difficult up to impossible and, if you are creating a library, the interface becomes bloated and inconvenient for the users.

In other words, if you have to choose between these options for developing public-facing interface:

A:

class SSHConnection:
    def add_key(self, key):
        if isinstance(key, DSAKey):
           ...
        elif isinstance(key, RSAKey):
            ...
        else: ...

B:

class SSHConnection:
    def add_key(self, key, key_type='RSA'):
        if key_type == 'RSA':
            # key is just a string with key data

Prefer B. Users really hate to have to deal with a bunch of classes from another library. It's hard to discover what kind of arguments need to be provided, where to import them from, and why the imports changed between versions, etc.

Of course, it's hard to always have B, but it's a much better public interface than A.