oop - イベントクラスの階層を改善するにはどうすればよいですか？

Question

スタックオーバーフローチャットのXMPPインターフェイスでは、チャットからのJSONフィードを解析し、送信されたメッセージ、送信された編集、ユーザーのログインまたはログアウトなど、すべてのチャットイベントに対してRubyオブジェクトを生成しています。 XMPPユーザーがStackOverflowチャットアカウントで認証できるようにするために、「/help」や「/auth」などの「コマンド」がXMPPサーバーに送信されます。

私はこれらのクラスを、論理的に理にかなっていると思う階層に設定しました。

class SOChatEvent # base class
 |
 |--- class SOXMPPEvent # base for all events that are initiated via XMPP
 | |
 | |--- class SOXMPPMessage # messages sent to the XMPP bridge via XMPP
 | | |
 | | |--- class SOXMPPMessageToRoom # messages sent from an XMPP user to an XMPP MUC
 | | |
 | | |--- class SOXMPPUserCommand # class for "slash commands", that is, messages starting
 | | | |                          # with /, used for sending commands to the bridge
 | | | |
 | | | |--- class SOXMPPUserHelpCommand
 | | | |--- class SOXMPPUserLoginCommand
 | | | |--- class SOXMPPUserBroadcastCommand
 |
 |--- class SOChatRoomEvent # base class for all events that originate from an SO chat room
 | |
 | |--- class SOChatMessage # messages sent to an SO chat room via the SO chat system
 | | |
 | | |--- class SOChatMessageEdit # edits made to a prior SOChatMessage
 | |
 | |--- class SOChatUserEvent # events related to SO chat users
 | | |
 | | |--- class SOChatUserJoinRoom #Event for when a So user joins a room
 | | |--- class SOChatUserLeaveRoom #Event for when a So user leaves a room

 (etc)

完全な階層とソースは、TracまたはSVNで確認できます。

私の質問は2つあります。まず、これらのイベントをインスタンス化するための最良の方法は何ですか？私が現在行っているのは、巨大なswitchステートメントを使用してJSONイベントを解析することです-まあ、それはルビーなのでcaseステートメントです-そして、それはまだ巨大ではありませんが、私がこのように続けるとそうなります：

rooms.each do |room|
  rid = "r"+"#{room.room_id}"
  if !data[rid].nil?
    @last_update = data[rid]['t'] if data[rid]['t']

    if data[rid]["e"]
      data[rid]["e"].each do |e|
        puts "DEBUG: found an event: #{e.inspect}"
        case e["event_type"]
          when 1
            event = SOChatMessage.new(room,e['user_name'])
            event.encoded_body = e['content']
            event.server = @server
            events.push event
          when 2
            event = SOChatMessageEdit.new(room,e['user_name'])
            event.encoded_body = e['content']
            event.server = @server
            events.push event
          when 3
            user = SOChatUser.new(e['user_id'], e['user_name'])
            event = SOChatUserJoinRoom.new(room,user)
            event.server = @server
            events.push event
          when 4
            user = SOChatUser.new(e['user_id'], e['user_name'])
            event = SOChatUserLeaveRoom.new(room,user)
            event.server = @server
            events.push event
        end
      end
    end
  end
end

しかし、私はそれを処理するためのより良い方法がなければならないと思います！のようなものSOChatEvent.createFromJSON( json_data )...しかし、適切なサブクラスのオブジェクトが特定の応答に応答して作成されるようにコードを構造化するための最良の方法は何event_typeですか？

第二に、私はまだ実際にはantサブクラスを使用してSOXMPPUserCommandいません。現在、すべてのコマンドはそれ自体の単なるインスタンスであり、そのクラスには、コマンドの正規表現に基づいて切り替わるSOXMPPUserCommand単一のメソッドがあります。executeほぼ同じ問題-私はより良い方法があることを知っています、私はただ最良の方法が何であるかわかりません：

def handle_message(msg)
    puts "Room \"#{@name}\" handling message: #{msg}"
    puts "message: from #{msg.from} type #{msg.type} to #{msg.to}: #{msg.body.inspect}"

    event = nil

    if msg.body =~ /\/.*/
      #puts "DEBUG: Creating a new SOXMPPUserCommand"
      event = SOXMPPUserCommand.new(msg)
    else
      #puts "DEBUG: Creating a new SOXMPPMessageToRoom"
      event = SOXMPPMessageToRoom.new(msg)
    end

    if !event.nil?
      event.user = get_soxmpp_user_by_jid event.from
      handle_event event
    end
  end

と：

class SOXMPPUserCommand < SOXMPPMessage
  def execute
    case @body
      when "/help"
        "Available topics are: help auth /fkey /cookie\n\nFor information on a topic, send: /help <topic>"
      when "/help auth"
        "To use this system, you must send your StackOverflow chat cookie and fkey to the system. To do this, use the /fkey and /cookie commands"
      when "/help /fkey"
        "Usage: /fkey <fkey>. Displays or sets your fkey, used for authentication. Send '/fkey' alone to display your current fkey, send '/fkey <something>' to set your fkey to <something>. You can obtain your fkey via the URL: javascript:alert(fkey().fkey)"
      when "/help /cookie"
        "Usage: /cookie <cookie>. Displays or sets your cookie, used for authentication. Send '/cookie' alone to display your current fkey, send '/cookie <something>' to set your cookie to <something>"
      when /\/fkey( .*)?/
        if $1.nil?
          "Your fkey is \"#{@user.fkey}\""
        else
          @user.fkey = $1.strip
          if @user.authenticated?
            "fkey set to \"#{@user.fkey}\". You are now logged in and can send messages to the chat"
          else
            "fkey set to \"#{@user.fkey}\". You must also send your cookie with /cookie before you can chat"
          end
        end
      when /\/cookie( .*)?/
        if $1.nil?
          "Your cookie is: \"#{@user.cookie}\""
        else
          if $1 == " chocolate chip"
            "You get a chocolate chip cookie!"
          else
            @user.cookie = $1.strip
            if @user.authenticated?
              "cookie set to \"#{@user.cookie}\". You are now logged in and can send messages to the chat"
            else
              "cookie set to \"#{@user.cookie}\". You must also send your fkey with /fkey before you can chat"
            end
          end
        end
      else
        "Unknown Command \"#{@body}\""
    end
  end
end

私はこれを行うためのより良い方法があることを知っていますが、それが具体的に何であるかはわかりません。SOXMPPUserCommandフォールのサブクラスを作成する責任はSOXMPPUserCommandそれ自体にあるべきですか？すべてのサブクラスを親に登録する必要がありますか？新しいクラスが必要ですか？

このような階層構造でサブクラスのオブジェクトをインスタンス化するための最良の方法は何ですか？

Answer 1

Addressing your first question. Here's some ideas you might like to consider

First, structure you sub-classes so they all use the same initiation parameters. Also, you could put some of the other initiating code there as well (such as your encoded_body and server accessors. Here's a skeleton of what I mean:

# SOChat Class skeleton structure
class SOChatSubClass  #< inherit from whatever parent class is appropriate
  attr_accessor :encoded_body, :server, :from, :to, :body

  def initialize(event, room, server)
    @encoded_body = event['content']
    @server = server
    SOChatEvent.events.push event

    #class specific code 
    xmpp_message = event['message']
    @from = xmpp_message.from
    @to = xmpp_message.to
    @body = xmpp_message.body
    #use super to call parent class initialization methods and to DRY up your code
  end
end 

Note that in my example you'll still have duplicated code in the sub-classes. Ideally you'd pull out the duplication by putting it in the appropriate parent class.

If you have problems creating a common list of initiation parameters, then rather than pass in a list of arguments (event, room, server), change the classes to accept an argument list as a hash {:event => event, :room => room, :server => server, etc}.

Regardless, once you have a common parameter structure for initializing the classes, you can initialize them a bit more dynamically, eliminating the need for the case statement.

class SOChatEvent
     class << self; attr_accessor :events; end
     @events = []

      @@event_parser = {
                                0 => SOChatSubClass, #hypothetical example for testing
                                1 => SOChatMessage,
                                2 => SOChatMessageEdit,
                                #etc
                              }
    def self.create_from_evt( json_event_data, room=nil, server=nil)
      event_type = json_event_data["event_type"]
      event_class =  @@event_parser[event_type]
      #this creates the class defined by class returned in the @@event_parser hash
      event_obj = event_class.new(json_event_data, room, server)
    end

    #rest of class
end

@@event_parser contains the mapping between event type and the class to implement that event type. You just assign the appropriate class to a variable and treat it just like the actual class.

Code like the following would create an object of the appropriate class:

event_obj = SOChatEvent.create_from_evt( json_event_data,
                                        "some room", 
                                        "some server")

Note: There are further optimizations that could be done to what I provided to be even cleaner and more concise, but hopefully this helps you get over the hump of the case statement.

Edit: I forgot to mention the Class instance variable SOChatEvent.events created with this: class << self; attr_accessor :events; end @events = []

You were pushing events to an event stack, but I wasn't clear where you wanted that stack to exist and whether it was a global events list, or specific to a particular class. The one I did is global, so feel free to change it if you wanted the event stack constrained to certain classes or instances.