refactor: small cleanup

[nstarman]

Slowly adding typing.
This would be good to combine with #147.

[coveralls]

Pull Request Test Coverage Report for Build 9419751162

Details

• 8 of 8 (100.0%) changed or added relevant lines in 1 file are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage remained the same at 99.919%

---

Totals
Change from base Build 9340121872:	0.0%
Covered Lines:	1241
Relevant Lines:	1242

---

💛 - Coveralls

[coveralls]

Pull Request Test Coverage Report for Build 9419952169

Details

• 16 of 16 (100.0%) changed or added relevant lines in 1 file are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+0.001%) to 99.92%

---

Totals
Change from base Build 9340121872:	0.001%
Covered Lines:	1246
Relevant Lines:	1247

---

💛 - Coveralls

[coveralls]

Pull Request Test Coverage Report for Build 9420047103

Details

• 16 of 16 (100.0%) changed or added relevant lines in 1 file are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+0.001%) to 99.92%

---

Totals
Change from base Build 9340121872:	0.001%
Covered Lines:	1246
Relevant Lines:	1247

---

💛 - Coveralls

[wesselb]

Thanks again for the PR, @nstarman! This change looks good to me.

One question about this: What’s the benefit of making Dispatcher a dataclass c.f. a normal class? Is it that you can set slots, presumably for a performance boost?

[PhilipVinc]

Slots means a tiny bit less overhead memory. But that does not impact anything we do because that’s sort of a singleton.

I like data classes when they can help make the codec more clear , but in general they are a pain to work with because you lose control over init …

‘I’m not sure it’s the best idea to use them here. Though in that particular case it won’t hurt either. But it can add friction for future refactors

[nstarman]

Slots means a tiny bit less overhead memory. But that does not impact anything we do because that’s sort of a singleton.

Only here. Dispatcher is part of the public API and other libraries create their own instances. Adding immutability and reducing memory are both nice additions.

I like data classes when they can help make the codec more clear , but in general they are a pain to work with because you lose control over init …

They reduce boilerplace, add introspection and interop tools (e.g. replace), and they don't relinquish control over __init__ (if an __init__ is defined on a class, not a superclass, @dataclass will not rewrite it).

[wesselb]

Thanks for the discussion, both! Since it’s a fairly small change and comes with some benefits (albeit minor ones), I think it’s worthwhile trying to make Dispatcher a dataclass. Should we later decide that we don’t like it after all, because it could make future changes more difficult as @PhilipVinc points out, it would be easy to make Dispatcher a normal class again. Will merge right away! :) Thanks again, @nstarman!