2009/9/15 [email protected] <[email protected]>: > Perdon .. pero --> jjajajaja nada mas feo que tener que estarse corrigiendo > a cada rato porque mandas sin darte cuenta.
Es peor, me corrigio por irc un amigo que lee la lista xD > Nicolás Sanguinetti wrote: >> >> 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 >> > > _______________________________________________ > 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
