Hace muchos años q no uso irc... Hay algo q valga la pena?

----
Dwayne Macgowan
Sent from mobile

On 15/09/2009, at 00:57, Nicolás Sanguinetti <[email protected]> wrote:

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 impl ementació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, m e 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 co n pedazos
en otros idiomas.

Imaginate si mañana buscás una librería que soluciona un pro blema 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 e scritos 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í simplement e '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 de volver
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*. E rgo, 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 debatib le :))



  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áci l 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 t ema de funcionalidad compartida, es fácil de cambiar. O directament e, definí la funcionalidad compartida en el módulo Packet, y hace incl ude 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
_______________________________________________
Ruby mailing list
[email protected]
http://lista.rubyargentina.com.ar/listinfo.cgi/ruby-rubyargentina.com.ar

Responder a