4
\$\begingroup\$

I created a Kik messenger bot using Flask microframework and python that accepts POST requests on the provided webhook and then responds appropriately. I have excluded custom module imports because they are not relevant in my problem.

import os
from flask import Flask, request, Response
from flask import Flask, request, Response, url_for
from kik import KikApi, Configuration
from kik.messages import LinkMessage, PictureMessage, VideoMessage
from kik.messages import StartChattingMessage, ScanDataMessage, StickerMessage
from kik.messages import SuggestedResponseKeyboard, TextResponse
from kik.messages import messages_from_json 


app = Flask(__name__)
BOT_USERNAME = os.environ['BOT_USERNAME']
BOT_API_KEY = os.environ['BOT_API_KEY']
WEBHOOK = os.environ['WEBHOOK']

features = {'manuallySendReadReceipts': False,
            'receiveReadReceipts': True,
            'receiveDeliveryReceipts': False,
            'receiveIsTyping': False}

static_keyboard = SuggestedResponseKeyboard(
    responses=[TextResponse('PLACEHOLDER1'),
               TextResponse('PLACEHOLDER2')
               ])

kik = KikApi(BOT_USERNAME, BOT_API_KEY)
kik.set_configuration(Configuration(
    webhook=WEBHOOK, static_keyboard=static_keyboard, features=features))


@app.route('/incoming', methods=['POST'])
def incoming():
    if not kik.verify_signature(request.headers.get('X-Kik-Signature'),
                                request.get_data()):
        return Response(status=403)

    messages = messages_from_json(request.json['messages'])

    for message in messages:
        user = message.from_user
        chat_id = message.chat_id

        if isinstance(message, LinkMessage):
            # do something

        elif isinstance(message, PictureMessage):
            # do something

        elif isinstance(message, VideoMessage):
            # do something

        elif isinstance(message, StartChattingMessage):
            # do something

        elif isinstance(message, ScanDataMessage):
            # do something

        elif isinstance(message, StickerMessage):
            # do something

        elif isinstance(message, TextMessage):
            query = message.body
            query_words = query.split()
            chat_type = message.chat_type
            participants = message.participants

            if query.lower() in ['placeholder1', 'placeholder2']:
                # do something

            elif query_words[0] in ['placeholder3', 'placeholder4']:
                # do something

            elif query_words[0] in ['placeholder5'] and user in ['mamun']:
                # do something

            # a large number of more elif statements present in actual app
            else:
                # do something

    return Response(status=200)


if __name__ == '__main__':
    app.run(port=8000, debug=True)

What I have done so far is create separate modules for custom functionality that I created, but in the end they are consumed by the large number of elif statements which I don't like. Is there any way I can avoid the elif statement hell?

My actual bot is 100% complete, production ready and approved by kik, but the way I had to program it still bothers me when I look at it.

\$\endgroup\$
  • 1
    \$\begingroup\$ You can simplify the elifs by using just one if. I would write an answer, but I am confused by the removed sections of code. Do you want the passes? \$\endgroup\$ – EugeneProut Sep 25 '17 at 13:22
  • \$\begingroup\$ I use functions imported from my own modules in place of pass es. I removed them to make it more clear as to what I am trying to achieve. \$\endgroup\$ – mamun Sep 25 '17 at 13:49
1
\$\begingroup\$

You could just build a dictionary mapping classes to functions to call:

class A:
    pass

class B:
    pass

def func_a(x, y):
    return x + y

def func_b(x, y):
    return x - y

if __name__ == "__main__":
    d = {LinkMessage: link_message,
         PictureMessage: picture_message,
         VideoMessage: video_message,
         StartChattingMessage: chatting_message,
         ScanDataMessage: data_message,
         StickerMessage: sticker_message}
    message = PictureMessage()
    funcs[message.__class__]()

Alternatively, if these classes are controlled by you, you could give them all a method called the same name which will be called, maybe handle_message:

class LinkMessage:
    ...
    def handle_message(self, *args):
        pass

if __name__ == "__main__":
    message = LinkMessage()
    message.handle_message(1, 2)
\$\endgroup\$
1
\$\begingroup\$

You are importing from the packages multiple times. Combine them into a single set:

import os

from flask import Flask, request, Response, url_for

from kik import KikApi, Configuration
from kik.messages import (LinkMessage, PictureMessage, VideoMessage,
    StartChattingMessage, ScanDataMessage, StickerMessage,
    SuggestedResponseKeyboard, TextResponse,
    messages_from_json)

Instead of referencing environment variables directly as key of dict, use the .get call or better yet, os.getenv method. This will not raise an error if the variable is not set.


When searching for substrings inside lists, better to search them inside a set (or tuple). The lookups are quicker in a set than list.

if query.lower() in ('placeholder1', 'placeholder2'):
# or
if query.lower() in {'placeholder1', 'placeholder2'}:

Additionally, if these values are not dynamically retrieved, you should store them as variables outside of the function body. This will also reduce the creation penalty you might face.


You could reduce the if-else nesting by following the advice @Graipher has in the other answer. For a more verbose cleanup, you might have to provide snippets belonging to those if-else blocks.

\$\endgroup\$
  • \$\begingroup\$ ('placeholder1', 'placeholder2') is not actually a set, as you say in your comment. It should be {'placeholder1', 'placeholder2'}. However, for such small things the construction time of a set over a simple tuple might not be worth it. \$\endgroup\$ – Graipher Oct 26 '17 at 13:56
  • \$\begingroup\$ @Graipher Thanks for pointing that out. Fixed and added another point for the construction penalty. \$\endgroup\$ – hjpotter92 Oct 26 '17 at 16:22

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.