dispatcher.add_handlers method #2813

david-shiko · 2021-12-11T02:51:42Z

david-shiko
Dec 11, 2021

What you think about dispatcher.add_handlers method?
For adding multiple handlers at once? It may increse perfomance and clarity a bit.

Here is differ implementations


dispatcher.add_handlers(
    handlers={
        -10: [
            dispatcher.add_handler(handler=handlers_definition.post_mass_sending_cmd),
            dispatcher.add_handler(handler=handlers_definition.post_mass_sending_cmd),
            dispatcher.add_handler(handler=handlers_definition.post_mass_sending_cmd),
            ],
        0: [
            dispatcher.add_handler(handler=handlers_definition.help_cmd),
            dispatcher.add_handler(handler=handlers_definition.help_cmd),
            dispatcher.add_handler(handler=handlers_definition.help_cmd),
            dispatcher.add_handler(handler=handlers_definition.help_cmd),
            ],
        1: [
            dispatcher.add_handler(handler=handlers_definition.faq_cmd),
            ],
        2: [
            dispatcher.add_handler(handler=handlers_definition.get_post_cmd),
            dispatcher.add_handler(handler=handlers_definition.get_post_cmd),
            dispatcher.add_handler(handler=handlers_definition.get_post_cmd),
            dispatcher.add_handler(handler=handlers_definition.get_post_cmd),
            ],
    }
)


dispatcher.add_handlers(  # Multiple dict keys with a same name!
    handlers={
        -10: dispatcher.add_handler(handler=handlers_definition.post_mass_sending_cmd),
        -10: dispatcher.add_handler(handler=handlers_definition.post_mass_sending_cmd),
        -10: dispatcher.add_handler(handler=handlers_definition.post_mass_sending_cmd),
        -10: dispatcher.add_handler(handler=handlers_definition.post_mass_sending_cmd),
        0: dispatcher.add_handler(handler=handlers_definition.help_cmd),
        0: dispatcher.add_handler(handler=handlers_definition.help_cmd),
        0: dispatcher.add_handler(handler=handlers_definition.help_cmd),
        1: dispatcher.add_handler(handler=handlers_definition.faq_cmd),
        2: dispatcher.add_handler(handler=handlers_definition.get_post_cmd),
        2: dispatcher.add_handler(handler=handlers_definition.get_post_cmd),
        2: dispatcher.add_handler(handler=handlers_definition.get_post_cmd),
    }
)


class Handler:
    pass


dispatcher.add_handlers(
    handlers=[
        Handler(handler=handlers_definition.post_mass_sending_cmd, group=-10),
        Handler(handler=handlers_definition.post_mass_sending_cmd, group=-10),
        Handler(handler=handlers_definition.post_mass_sending_cmd, group=-10),
        Handler(handler=handlers_definition.post_mass_sending_cmd, group=-10),
        Handler(handler=handlers_definition.help_cmd, group=0),
        Handler(handler=handlers_definition.help_cmd, group=0),
        Handler(handler=handlers_definition.help_cmd, group=0),
        Handler(handler=handlers_definition.faq_cmd, group=0),
        Handler(handler=handlers_definition.get_post_cmd, group=1),
        Handler(handler=handlers_definition.get_post_cmd, group=1),
        Handler(handler=handlers_definition.get_post_cmd, group=1),
    ]
)

For example look at this non optimal code from dispatcher.add_handler:

        if group not in self.handlers:
            self.handlers[group] = []
            self.groups.append(group)
            self.groups = sorted(self.groups)

        self.handlers[group].append(handler)

BTW I still don't understand why need self.groups if we may iterate over a self.handlers keys

Bibo-Joshi · 2021-12-11T08:53:35Z

Bibo-Joshi
Dec 11, 2021
Maintainer

Hi. thinks for the suggestion. The first one seems reasonable to me 👍 @Poolitzer @harshil21 what do you think?

Note that the second one can't work since e.g. {1: 1, 1:2} results in {1: 2}. Making the group an attribute of the handler does not seem like a good idea to me - the groups are a logic specific to the Dispatcher and a single handler alone can do nothing with this information.

BTW I still don't understand why need self.groups if we may iterate over a self.handlers keys

I'm not sure, tbh. That was added before my time at PTB …

0 replies

harshil21 · 2021-12-11T10:04:20Z

harshil21
Dec 11, 2021
Collaborator

The first one looks great, but why use dispatcher.add_handler inside it when one can just pass the handler object directly anyway? We can eliminate more boilerplate.

One drawback is that people who are using only the default group will still have to write the 0: [...] part. In that case we can also make the handlers parameter accept a list of Handlers (setting default group of 0)

6 replies

harshil21 Dec 13, 2021
Collaborator

Yes, add_handlers(handlers, group) (single method) is what I'd go for.

Bibo-Joshi Dec 13, 2021
Maintainer

Okay. @david-shiko would you like to PR for this? A PR should be based on the v14 branch and follow the contribution guide. For the tests to add, you can get inspiration e.g. here.
Please let me know if you're interested :)

Poolitzer Dec 15, 2021
Collaborator

+1 for the single function. Maybe create an issue for this? Especially if OP doesn't respond.

david-shiko Dec 16, 2021
Author

@Bibo-Joshi I don't know. My pet project may take a time. Did you suppose a particulary deadline?

Bibo-Joshi Dec 16, 2021
Maintainer

All right - no worries :) I don't have a deadline in mind. It's a non-breaking addition, so it doesn't even have to be in v14. I'll just open a corresponding issue and if it's still open when you have time for it, you're welcome to PR :)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

dispatcher.add_handlers method #2813

Uh oh!

{{title}}

Uh oh!

Uh oh!

{{editor}}'s edit

{{editor}}'s edit

Uh oh!

Replies: 2 comments 6 replies

Uh oh!

{{title}}

Uh oh!

Uh oh!

{{title}}

Uh oh!

Uh oh!

{{editor}}'s edit

{{editor}}'s edit

Uh oh!

Uh oh!

{{title}}

Uh oh!

Uh oh!

{{title}}

Uh oh!

Uh oh!

{{title}}

Uh oh!

Uh oh!

{{title}}

Uh oh!

Uh oh!

{{title}}

Uh oh!

Select a reply

Uh oh!

dispatcher.add_handlers method #2813

Uh oh!

Uh oh!

david-shiko Dec 11, 2021

Replies: 2 comments · 6 replies

Uh oh!

Bibo-Joshi Dec 11, 2021 Maintainer

Uh oh!

Uh oh!

harshil21 Dec 11, 2021 Collaborator

Uh oh!

harshil21 Dec 13, 2021 Collaborator

Uh oh!

Bibo-Joshi Dec 13, 2021 Maintainer

Uh oh!

Poolitzer Dec 15, 2021 Collaborator

Uh oh!

david-shiko Dec 16, 2021 Author

Uh oh!

Bibo-Joshi Dec 16, 2021 Maintainer

david-shiko
Dec 11, 2021

Replies: 2 comments 6 replies

Bibo-Joshi
Dec 11, 2021
Maintainer

harshil21
Dec 11, 2021
Collaborator

harshil21 Dec 13, 2021
Collaborator

Bibo-Joshi Dec 13, 2021
Maintainer

Poolitzer Dec 15, 2021
Collaborator

david-shiko Dec 16, 2021
Author

Bibo-Joshi Dec 16, 2021
Maintainer