dispatcher.add_handlers method

[david-shiko]

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]

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 …

[harshil21]

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)

[harshil21]

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

[Bibo-Joshi]

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]

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

[david-shiko]

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

[Bibo-Joshi]

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 :)