Lo que sale de self.read camina y hace cuack como pato no?
Entonces tiene que ser un pato, aunque se vea un poco feo asi empaquetado.
Porque tengo que quedar enganchado a Packet?
Eh, por cierto, FAIL mio, tendría que ser algo como esto:
>
> module Packet
> BINARY_PACKET_TYPE = 224
> ASCII_PACKET_TYPE = 36
>
> def self.read(message)
> case message
> when BINARY_PACKET_TYPE; Packet::Binary.new(message)
> when ASCII_PACKET_TYPE; Packet::Text.new(message)
> else raise ArgumentError, "Wrong packet type: #{message}"
> end
>
> class Binary
> end
>
> class Text
> end
> end
>
> O bueno, los nombres capaz que no son los mejores (de nuevo, ni idea
> que es OpenDMTP :)), pero no esta bueno tirar numeros "al azar" en el
> medio del codigo :)
>
> -f
>
> > Packet.read(...)
> >
> > Fijate como no tenés que andar haciendo aliases ni mapeando strings a
> > clases, y como me ahorro varios niveles de indirección.
> >
> >> Leyendo varios articulos encontrados en google respecto a la
> >> implementación del patron
> >> factory en ruby y jugando un poco con metaprogramación, me quedó el
> >> siguiente codigo:
> >>
> >> class Packet
> >>
> >> class << self
> >> alias :nuevo :new
> >>
> >> def inherited(subclass)
> >> class << subclass
> >> alias :new :nuevo
> >> end
> >> end
> >
> > Sobre esto:
> >
> > 1) (esta es a título personal) por favor, POR FAVOR, no escriban
> > nombres de métodos en español. Queda horrible leer código con pedazos
> > en otros idiomas.
> >
> > Imaginate si mañana buscás una librería que soluciona un problema X, y
> > empezás a usarla, y cuando te encontrás con un error y querés revisar
> > el código, los comentarios y la mitad de los métodos están escritos en
> > ruso? :-/
> >
> > (ojo, digo ruso como podría decir cualquier idioma, si justo sabés
> > ruso cambialo por otro :P)
> >
> > 2) En mi opinion personal, no esta bueno redefinir Class#new (en este
> > caso particular Packet.new). Prefiero mil veces agregar otro metodo de
> > clase que llame new. Pero ta, ponele que queres igual usar new, porque
> > te gusto, esto alcanza:
> >
> > class Packet
> > class << self
> > alias_method :new_without_factory, :new
> > end
> >
> > def self.new(*args)
> > # haces cosas
> > new_without_factory(*args)
> > end
> > end
> >
> > No tenes que usar self.inherited ni dar esas vueltas que te hacen leer
> > como 3 veces el código para saber que querés lograr.
> >
> >> def new(*args)
> >> msg = *args
> >> encoding = get_packet_encoding(msg)
> >> Kernel.const_get(encoding).new unless encoding == "Unknown"
> >> end
> >
> > Tiene sentido que Packet.new reciba más de un argumento? Porque
> > después cuando llamas BinaryPacket.new, o TextPacket.new, no les estás
> > pasando los args.
> >
> > No se si te falto un new(*args) o simplemente querés usar el primer
> > argumento para deducir el tipo de clase a usar. Si es lo segundo,
> > entonces usar splat-args no tiene sentido. Definí simplemente 'msg'
> > como parametro.
> >
> >> def get_packet_encoding(msg)
> >> encoding = case msg[0]
> >> when 224 then "BinaryPacket"
> >> when 36 then "TextPacket"
> >> else "Unknown"
> >> end
> >> end
> >> end
> >
> > Aca viene la parte que creo es más fea. Para qué querés devolver
> > strings y hacer const_get de esos strings, cuando podés devolver
> > directamente la clase que querés? Estás agregandole niveles de
> > indirección totalmente innecesarios.
> >
> > Y por último, "Unknown". Si yo hago Packet.new(42) con tu codigo,
> > devuelve nil. Eso es feo. Tener que andar chequeando por nil es Feo
> > (con f mayúscula).
> >
> > Vos querés que *no funcione*, entonces lo ideal es que el usuario
> > reciba una notificación de que lo que hace, *no funciona*. Ergo, una
> > excepción me parece una mejor idea. Cuál? Depende, si estás haciendo
> > una libreria capaz que definir tu propia excepción tipo class
> > WrongPacketTypeProvided < ArgumentError; end tiene sentido. Si no, con
> > tirar un ArgumentError está bien (en mi opinión, es debatible :))
> >
> >> def initialize
> >> puts "hola desde #{self.class}"
> >> end
> >> end
> >>
> >> class TextPacket < Packet
> >> end
> >>
> >> class BinaryPacket < Packet
> >> end
> >
> > Y ta, después, lo de hacer Packet::Text en lugar de TextPacket <
> > Packet es puramente un tema de estilo personal, dado lo fácil que es
> > en ruby de pisar cosas sin darte cuenta, prefiero poner todo en
> > namespaces.
> >
> > Si TextPacket y BinaryPacket heredaban de Packet por algún tema de
> > funcionalidad compartida, es fácil de cambiar. O directamente, definí
> > la funcionalidad compartida en el módulo Packet, y hace include Packet
> > adentro de las child classes.
> >
> >> No conocia la posibilidad de definir alias de metodos, la
> >> posibilidad de hacer modificaciones
> >> temporales en runtime "backupeando" el metodo con un alias es
> alucinante.
> >>
> >> Si uno le muestra esto a un programador que no conozca ruby, cuando
> >> entienda como
> >> funciona se le vuela el peluquín. :)
> >>
> >> Calculo que en un par de meses voy a tener una implementacion
> >> cliente-servidor de
> >> OpenDMTP decente basada en EventMachine, cuando la termine la voy a
> liberar.
> >>
> >> En fin, queria compartir mi "descubrimiento" con ustedes.
> >
> > Y ta, por último, no te lo tomes a mal, si el lenguaje te parece
> > violento o algo, creeme que no es intencional, cuando digo que no
> > entiendo nada de diplomacia, es cierto, simplemente te digo lo que me
> > parece haría a tu código mejor :)
> >
> > Y ta, obviamente, todo el mundo está invitado a demostrarme como
> > mejorar mi ejeplo :D
> >
> > <chivo> Los code reviews son divertidos, tu equipo puede mejorar su
> > calidad de código usando http://howsmycode.com :P </chivo>
> >
> > -foca
> >
> _______________________________________________
> Ruby mailing list
> [email protected]
> http://lista.rubyargentina.com.ar/listinfo.cgi/ruby-rubyargentina.com.ar
>
_______________________________________________
Ruby mailing list
[email protected]
http://lista.rubyargentina.com.ar/listinfo.cgi/ruby-rubyargentina.com.ar
