Comment puis-je améliorer la hiérarchie de mes classes d'événements?

Question

Pour le XMPP interface for the Stack Overflow chat, j'analyse le flux JSON du chat et génère des objets Ruby pour tous les événements de chat, tels que les messages envoyés, les modifications envoyées, les utilisateurs qui se connectent ou se déconnectent, etc. le serveur XMPP, comme "/ help" ou "/ auth" afin de permettre à l'utilisateur XMPP de s'authentifier avec son compte de discussion Stack Overflow.

J'ai mis en place ces classes dans une hiérarchie, je me sens fait bon sens logique:

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) 

Vous pouvez voir la hiérarchie complète et la source in Trac ou via SVN.

Ma question est double: Premièrement, quelle est la meilleure façon d'instancier ces événements? Ce que je suis en train de faire est d'analyser les événements JSON en utilisant une déclaration géant - bien, c'est ruby ​​donc c'est une déclaration case - et, ce n'est pas géant encore, mais ce sera si je continue comme ça:

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 

Mais j'imagine qu'il doit y avoir une meilleure façon de gérer ça! Quelque chose comme SOChatEvent.createFromJSON(json_data) ... Mais, quelle est la meilleure façon de structurer mon code afin que les objets de la sous-classe appropriée soient créés en réponse à un event_type? Deuxièmement, je n'utilise pas encore les sous-classes ant de SOXMPPUserCommand pour le moment. À l'heure actuelle, toutes les commandes ne sont que des instances de SOXMPPUserCommand elle-même, et cette classe a une seule méthode execute qui bascule en fonction de la regex de la commande. Il en va de même problème - je sais qu'il ya une meilleure façon, je ne suis pas sûr de ce que la meilleure façon est:

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 

et:

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 

Je sais qu'il ya une meilleure façon de le faire, Je ne sais pas exactement ce que c'est. Est-ce que la responsabilité de créer des sous-classes de SOXMPPUserCommand incombe à SOXMPPUserCommand lui-même? Est-ce que toutes les sous-classes doivent s'inscrire auprès du parent? Ai-je besoin d'une nouvelle classe?

Quelle est la meilleure façon d'instancier des objets de sous-classes dans une telle structure hiérarchique?

Answer 1

Répondre à votre première question. Voici quelques idées que vous pourriez envisager

D'abord, structurez vos sous-classes de sorte qu'elles utilisent toutes les mêmes paramètres d'initiation. En outre, vous pouvez mettre un peu de l'autre code initiation là aussi (comme votre accesseurs encoded_body et serveur est ici un squelette de ce que je veux dire.

# 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 

Notez que dans mon exemple, vous aurez encore le code dupliqué Dans l'idéal, vous devez supprimer la duplication en la plaçant dans la classe parente appropriée

Si vous rencontrez des problèmes lors de la création d'une liste commune de paramètres d'initialisation, alors plutôt que de transmettre une liste d'arguments (événement , room, server), changez les classes pour accepter une liste d'arguments comme hash {: event => event,: room => room,: server => serveur, etc}.Quoi qu'il en soit, une fois que vous avez une structure de paramètres commune pour l'initialisation des classes, vous pouvez les initialiser un peu plus dynamiquement, éliminant ainsi la nécessité de l'instruction case.

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 contient la correspondance entre le type d'événement et la classe pour mettre en oeuvre ce type d'événement. Vous affectez juste la classe appropriée à une variable et la traite comme la classe réelle.

code comme celui-ci créerait un objet de la classe appropriée:

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

Remarque: Il existe d'autres optimisations qui pourraient être faites à ce que je fournis pour être encore plus propre et plus concis, mais nous espérons que cela aide vous obtenez sur la bosse de la déclaration de cas.

Edit: j'oublié de mentionner l'instance de classe Variable SOChatEvent.events créée avec ceci: class << self; attr_accessor :events; end @events = []

Vous poussiez des événements à une pile d'événements, mais je ne sait pas où vous vouliez que la pile d'exister et si elle était une liste d'événements globale, ou spécifique à une classe particulière. Celui que j'ai fait est global, alors n'hésitez pas à le changer si vous voulez que la pile d'événements soit limitée à certaines classes ou instances.