2009/9/15 Nicolás Sanguinetti <[email protected]>:
> 2009/9/15 Nicolás Sanguinetti <[email protected]>:
>> 2009/9/14 Sebastian A. Espindola <[email protected]>:
>>> Gente,
>>> Estaba tirando código para un proyecto personal (una implementación
>>> en Ruby de
>>> un server del protocolo OpenDMTP) y me encontre con la necesidad de
>>> que una clase
>>> interprete si los datos que recibe desde un socket estan en un
>>> formato binario o texto
>>> y genere una instancia que actue sobre esa codificación.
>>
>> Hola, me llamo Nicolás y no entiendo nada de diplomacia. Tu solucion
>> es horrible.
>>
>> Sin saber bien lo que estas haciendo (lease, no se que es OpenDMTP) lo
>> que yo haría (y después, abajo, comento tu código parte por parte.
>
> 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
Ta, doble FAIL: me falto el `end` que cierra el `case message`.
Pero ta, se entiende la idea :)
> 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
