Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove if condition in _run_handlers #11

Closed
sekarpdkt opened this issue Dec 25, 2018 · 7 comments
Closed

Remove if condition in _run_handlers #11

sekarpdkt opened this issue Dec 25, 2018 · 7 comments

Comments

@sekarpdkt
Copy link

Hi

The if condition in _run_handlers is creating issues, when I try to test tgvoip example. I modified it as


    def _run_handlers(self, update: Dict[Any, Any]) -> None:
        if update.get('@type') == 'updateNewMessage':
            for handler in self._message_handlers:
                self._workers_queue.put(
                    (handler, update),
                    timeout=self._queue_put_timeout,
                )
        else:
            
            for handler in self._message_handlers:
                self._workers_queue.put(
                    (handler, update),
                    timeout=self._queue_put_timeout,
                )

or we can create another method for add_message_handler like add_call_handlerand append functions to new list like _call_handler. In else condition use it.

If ok, I can create a PR.

@alexander-akhmetov
Copy link
Owner

alexander-akhmetov commented Dec 27, 2018

Hi,

or we can create another method for add_message_handler like add_call_handlerand append functions to new list like _call_handler. In else condition use it.

It would be nice to have this! Maybe it's better to name this method like add_update_handler and not to call it in the else condition, but every time for every update (there is already an _update_handlers list for that):

   
    def add_update_handler(self, func: Callable):
       # later here we can add a second parameter: type
       # to register handlers for specific types
        if func not in self._update_handlers:
            self._message_handlers.append(func)

    def _run_handlers(self, update: Dict[Any, Any]) -> None:
        if update.get('@type') == 'updateNewMessage':
            for handler in self._message_handlers:
                self._workers_queue.put(
                    (handler, update),
                    timeout=self._queue_put_timeout,
                )
        
        for handler in self._update_handlers:
                self._workers_queue.put(
                    (handler, update),
                    timeout=self._queue_put_timeout,
                )

If you have time, you can create a PR, if not, I will assign this issue to myself. :)

@sekarpdkt
Copy link
Author

Ok. Thanks for suggestion. Will do it this weekend and let u know.

@sekarpdkt
Copy link
Author

I did this way. If ok, will send a PR

self._update_handlers: List[Callable] = {}

I declared it as a dictionary. Then


    def _run_handlers(self, update: Dict[Any, Any]) -> None:
        if update.get('@type') == 'updateNewMessage':
            for handler in self._message_handlers:
                self._workers_queue.put(
                    (handler, update),
                    timeout=self._queue_put_timeout,
                )
        try:
            for handler in self._update_handlers[update.get('@type')]:
                    self._workers_queue.put(
                        (handler, update),
                        timeout=self._queue_put_timeout,
                    )
                    print(update.get('@type'))

        except KeyError:
            pass;
        
    
   
    def add_update_handler(self, updateType,func: Callable):
        try:
            if func not in self._update_handlers[updateType]:
                self._update_handlers[updateType].append(func)
        except KeyError:
            self._update_handlers[updateType]=[]
            self._update_handlers[updateType].append(func)
            
    def add_message_handler(self, func: Callable) -> None:
        if func not in self._message_handlers:
            self._message_handlers.append(func)

@alexander-akhmetov
Copy link
Owner

👍 Overall it looks nice, exactly what I meant in the comment # later here we can add a second parameter: type to register handlers for specific types.

I would try to avoid big try-except block in the _run_handlers method and used self._update_handlers.get(update_type, []) instead. And I think add_message_handler can add functions to the _update_handlers with a relevant type.

@sekarpdkt
Copy link
Author

Ok.. got the point. As add update will be called only once, it should be ok. Where as run handlers will handle all calls, keeping it light weight is what you mean, I think.


    def _run_handlers(self, update: Dict[Any, Any]) -> None:

        for handler in self._update_handlers.get(update.get('@type'),[]):
                self._workers_queue.put(
                    (handler, update),
                    timeout=self._queue_put_timeout,
                )

    def add_update_handler(self, updateType,func: Callable):
        try:
            if func not in self._update_handlers[updateType]:
                self._update_handlers[updateType].append(func)
        except KeyError:
            self._update_handlers[updateType]=[]
            self._update_handlers[updateType].append(func)
            
    def add_message_handler(self, func: Callable) -> None:
        self.add_update_handler('updateNewMessage',func);
            

@alexander-akhmetov
Copy link
Owner

Yes, exactly :) Also, change updateType to type, please :) (Maybe it will be more convenient to continue in the PR)

@alexander-akhmetov
Copy link
Owner

Fixed in #19

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants