On Wed, Jul 15, 2009 at 06:59:12PM -0800, Michael Fowler wrote:
> On Wed, Jul 15, 2009 at 04:08:48AM -0400, Rocco Caputo wrote:
> > For starters, $_[ARG0] isn't guaranteed to contain anything in particular.
> 
> Well, it's guaranteed to contain whatever was passed as the 'args'
> argument to the POE::Session constructor, right?  When the session is
> constructed, 'args' is [ $socket, $client_args ].  That 'args' value
> needs to remain, because the POE::Wheel::ReadWrite constructor needs it.
> Having ClientConnected then mirror _start in terms of arguments seems
> reasonable.
> 
> 
> > I think you're almost right.  The correct behavior would be for  
> > ClientArgs to align with @_[ARG0..$#_].  This still breaks current code, 
> > but it's cleaner.
> 
> Well, I don't mind breaking old code, but I think ARG0 being the socket
> should be preserved.  Reaching into $heap->{client}->get_input_handle
> seems awkward.  Then again, perhaps it's breaking an abstraction to have
> direct access to the socket outside of POE::Wheel::ReadWrite.
> 
> The code I have that actually needs it would benefit from the clarity of
> having the socket passed as ARG0.  Whether or not this breaks some
> possible future change in something...
> 
> --
> Michael Fowler
> www.shoebox.net

First off, thanks for all the patches, I've applied them all apart
from this one.

And the main reason for that is deciding what is the best course of
action.

( I have no vested interest in POE::Component::Server::TCP myself
  so can have a non-partisan view ).

There is a difference between the documented behaviour and the actual 
behaviour.

I, personally, can't see the need to pass the socket object to the
ClientConnected handler, and without any evidence whatsoever to 
substantiate my claim >:) I'd suggest that no one actually uses 
the socket.

ClientConnected would be used to send any initial text to the client
which one would usually use $_[HEAP]{client} for.

Similarly, one might want to find out the TCP connection details,
which are again in the heap $_[HEAP]{remote_ip}, etc.

My pennyworth.

Anyways, I see three options:

a). Make the functionality match the documentation;

b). Make the documentation match the functionality;

c). Do what dngor suggests and flatten ClientArgs, forget the socket
    and document as such.

Cheers,

-- 
Chris Williams
aka BinGOs
PGP ID 0x4658671F
http://www.gumbynet.org.uk
==========================

Attachment: pgpvxPjGYdYp5.pgp
Description: PGP signature

Reply via email to