Fixed #30801 -- Improved guidance for making good use of signals.

[jvzammit]

Ticket

#30801

Doubts I have

• In ref/signals the warning sections are located after the sections about the signals the warning is about. I continued to follow this pattern. Should the user be warned ahead of using the signal? Not sure.
• In topics/signals the setting_changed usage example repeats a lot of code from the Where should this code live? section further down. I tried to minimise the repetition. But I'm not sure I did a good enough job. Feedback appreciated!

cc @adamchainz

[carltongibson]

Hey @jvzammit — thanks for this.

I pushed a couple of commits with edits. Can you let me know what you think?

I rephrased the model warning one, and moved to the top of that section. The rest was just tweaks.

🎁

[jvzammit]

@carltongibson All edits are good, thanks!

I rephrased the model warning one, and moved to the top of that section.

Nice!

I have a question about this:

Signals can make your code harder to maintain. Consider implementing a
helper method on a :ref:custom manager <custom-managers>, to
both update your models and perform additional logic, or else
:ref:overriding model methods <overriding-model-methods> before using
model signals.

I see how overriding model methods is a better (in my opinion) approach. E.g. replacing signal usage with model instance methods is obvious to me. For example overriding save.

However I do not find it that clear how custom managers would make signal usage redundant. Perhaps because I always see signals used at the "instance" rather than "queryset" level.

After writing the above part (which I've striked through) I realised you are referring to this example use case: I want to send an email when a customer is activated:

class CustomerManager(models.Manager):

    def activate(self):
        self.get_queryset().update(is_active=True)
        # send "you're account is now active" email to customers in queryset

As opposed to a post_save signal.

So I don't think there is anything else to add. Thanks again 👍 👍

[carltongibson]

Also, it may be that a manager is a better home for some logic, often when involving multiple instances where there's no clear parent.

[adamchainz]

Small comments but otherwise LGTM

[adamchainz]

No hyphen required

also I would argue they do provide loose(r) coupling, not just the appearance thereof?

Suggested change

	Signals give the appearance of loose-coupling, but they can quickly lead to
	Signals give the appearance of loose coupling, but they can quickly lead to

[carltongibson]

I don't think the code is really decoupled 😅 Rather it's just somewhere else. Not the same thing. 😜

[jvzammit]

They result in apparent loose coupling. In my opinion, the resulting code is tightly coupled. But usually located in another module where I wouldn't intuitively look for it.

"Loose coupling" definition:

In computing and systems design, a loosely coupled system is one in which components are weakly associated with each other, and thus changes in one component least affect existence or performance of another component

"Coupling" definition:

coupling is the degree of interdependence between software modules; a measure of how closely connected two routines or modules are

I believe that a full model definition includes the signals defined for it. Or that's how I would like to think about it. Signals make that harder. As opposed to the alternatives suggested (as manager functions, overriding instance functions).

That's why they are coupled but give the impression of loose coupling.

[jvzammit]

Thanks for the feedback @adamchainz ! I added commits with both of your suggestions.
And unresolved this thread, so we can agree on it.

[adamchainz]

Fair enough, I don’t want to split hairs on this! Good work all!