Refactor Command to Type Logic

[joshuaskelly]

Summary

Rather than constructing a new command to type dictionary each time an event is init'd, we should cache the dictionary.

[PythooonUser]

How should this be done?

1. Via a class variable in Event?
2. Via a class method in EventType?
3. Via something else?

[joshuaskelly]

I like 2 a class method in EventType. Does this mean you are considering working on this?

[PythooonUser]

Sure, I could do it. Or do you want to work on this? :P I think I understand the issue sufficient enough^^

[joshuaskelly]

You were talking implementation details. 😛

You can take it, or @Senpos since they feel strongly about it.

[senpos]

I will do it tomorrow, if you don't mind. Or you can do it on your own in case you want to publish these changes to PyPI ASAP.

[PythooonUser]

@Senpos Please, go ahead :)

[senpos]

I like 2 a class method in EventType.

How should this look like?

Well, I could do something like this:

class TwitchChatEventType(object):
    TWITCHCHATJOIN = 'JOIN'
    TWITCHCHATLEAVE = 'PART'
    TWITCHCHATMESSAGE = 'PRIVMSG'
    TWITCHCHATMODE = 'MODE'
    TWITCHCHATCLEARCHAT = 'CLEARCHAT'
    TWITCHCHATHOSTTARGET = 'HOSTTARGET'
    TWITCHCHATNOTICE = 'NOTICE'
    TWITCHCHATRECONNECT = 'RECONNECT'
    TWITCHCHATROOMSTATE = 'ROOMSTATE'
    TWITCHCHATUSERNOTICE = 'USERNOTICE'
    TWITCHCHATUSERSTATE = 'USERSTATE'
    TWITCHCHATWHISPER = 'WHISPER'
    TWITCHCHATCOMMAND = 'COMMAND'

    @classmethod
    def from_command(cls, command):
        if hasattr(cls, command):
            return getattr(cls, command)
        return cls.TWITCHCHATCOMMAND

class TwitchChatEvent(object):
    def __init__(self, channel=None, command=None, message=''):
        self.type = TwitchChatEventType.from_command(command)
        ...

But this relies on the way how @PythooonUser will change the naming. So I'd postpone this change.

Also, this is not the best way to gain performance. Yes, it is way better than it is now, but worse than command_to_type dict in module / class level.

Thoughts?

[PythooonUser]

I like it. It keeps the logic tidy.

[joshuaskelly]

Did you perf test it?

[joshuaskelly]

I wrote a set of performance tests to see which various implementations are fastest (which is the whole reason we are doing this work).

https://gist.github.com/joshuaskelly/cc1e11cd19f2f55209f52d25b8aa18c9

My Results:

Cached dictionary:
2.0602000000000004e-07

Getattr:
3.1746e-07

If statement:
2.8941e-07

This is measuring average time per call.

[senpos]

@joshuaskelly My mistake, using hasattr / getattr is not appropriate way to make it work right. dict has keys that represent actual Twitch commands, but EventType has readable values of the types that are used inside the bot.

So, should we stick with the fastest way, which is mapping of Twitch commands to the actual internal EventType values and place it at the module level? (variant "a" from your measurement).

According to the latest @PythooonUser PR:

_COMMAND_TO_TYPE = {
    'JOIN': EventType.JOIN,
    'PART': EventType.LEAVE,
    'PRIVMSG': EventType.MESSAGE,
    'MODE': EventType.MODE,
    'CLEARCHAT': EventType.CLEARCHAT,
    'HOSTTARGET': EventType.HOSTTARGET,
    'NOTICE': EventType.NOTICE,
    'RECONNECT': EventType.RECONNECT,
    'ROOMSTATE': EventType.ROOMSTATE,
    'USERNOTICE': EventType.USERNOTICE,
    'USERSTATE': EventType.USERSTATE,
    'WHISPER': EventType.WHISPER
}

class Event:
    def __init__(...):
        _COMMAND_TO_TYPE.get(command, EventType.COMMAND)

[PythooonUser]

Is this still being worked on?

[senpos]

@PythooonUser I am not sure, waiting for you and @joshuaskelly to choose the proper variant, described above :)