Re: [PEPr] +1 for Networking::Net_Gearman

On Fri, 2008-25-04 at 09:12 -0700, Joe Stump wrote: > On Apr 24, 2008, at 4:15 PM, Michael Gauthier wrote: > > > - $multiByteSupport should be private static instead of public > > Done > > > > > - documentation of connect() says it populates the static $magic > > array but it actually happens in immediately executable code after > > the class definition. Could the initialization of the $magic array > > happen lazily like the static $multiByteSupport member variable? > > That way both the commands and magic properties could also be > > private. Initilization could be done with a private static method > > and reused in the other Connection methods. > > Since all of the other methods require a connection to even work I put > it in Net_Gearman_Connection::connect(). Putting it in a different > method and then calling it from each function would add unnecessary > function overhead (which is substantial in PHP unfortunately) to a > package that's meant specifically for scalability and speed. Hopefully > this is a fair compromise. > > It should also be noted here that, if I were not using PHP, the > connection class would be a "private" class. It's not meant for people > to use willy nilly on it's own. > > > - documentation of magic refers to non-existent constructor > > - connect() method refers to non-existent $socket member variable > > - connect() documentation says it returns void but it really returns > > a socket resource > > Cleaned up. > > > - this may not be applicable to Gearman but you should be able to > > specify the port in the connect() method > > You can by passing '127.0.0.1:7007' to $host. > > > - @throws documentation should say why an exception would be thrown > > Added this. This, btw, is not documented on phpdoc.org. Someone needs > to poke Josh Eichorn. :) > > I'll have this and Travis's input ready to roll for the initial 0.1.0 > release. > > Thanks for the input! :) > > --Joe > Sounds good to me. I consider the conditional vote conditions met! :) Mike