cgi-session-user

Re: [Cgi-session-user] proposed refinements to load() and find() interfaces

[Matt L. <mle...@cp...>]

On 6/10/06, *Mark Stosberg* <ma...@su...> wrote:

    "load" vs "_load" is not clear.


load is part of the documented public interface, _load is not. This is a 
change to internals, nothing else.

    I propose instead the following:

    1. A named-parameters interface for load():

      load(
            dsn      => $dsn,
            dsn_args => \%dsn_args,
            query    => $query,
            update_atime => 1, # defaults to 1
      );

    2. And also a named-parameters interface for find()

    find(
       dsn       => $dsn,
       dsn_args  => \%dsn_args,
       code      => inspect( look => 'closely'),
       update_atime => 0, # defaults to 0 here!
    )

    Notice that "dsn" and "dsn_args" are now together, which is nice. Also
    notice that the "coderef_args" is not needed. Instead, higher order
    functions can used, which have worked well in the Data::FormValidator
    4.0 interface. If coderefs args would be needed, a closure can be
    returned whic provides them. Here's an example what "inspect()" might
    look like:

            sub inspect {
                    my %args = @_;
                    return sub {
                            my $session = shift;
                            my $coderef_args =  \%args;
                            # do something...

                    }
            }


The point of the change that I'm making is to allow us to inspect the 
session as the user has left it without changing anything. I strongly 
feel that the current behavior of find is wrong. find is supposed to 
manage the sessions as the user has left them. If you wish to add 
something to the session, no problem. Call param on the session as you 
normally would and the changes will be made but the access time will 
not. If someone really wants to update the access time, then they can 
either load the session or we can add an update_atime method.

    Matt,

    If you agree with these refactorings, would you mind coding them up?


I wouldn't mind coding them if I agreed with them =)

-Matt

Re: [Cgi-session-user] proposed refinements to load() and find() interfaces

[Matt L. <mle...@cp...>]

On 6/10/06, *Mark Stosberg* <ma...@su...> wrote:
> Here's another refactor:
>
> 1. Leave things generally alone with load(), but add extra param,
>   for internal use only, just be clearer than load/_load
>
>   load($dsn,$query, \%dsn_args, $update_atime);
>
All right. _load's been ditched and load now accepts a fourth parameter. 
No documentation has been added about the fourth option and should be 
considered for internal use only.
> 2. Leave find() mostly alone as well, and don't add update_atime flag.
>    Back out the coderef_args option patch in favor of documenting how
>    to use a higher order function for this.
>
>   find( $dsn,
>                 inspect( look => 'closely'), # will return a code ref
>         \%dsn_args );
Ok then. find is now changed back to three args (sorry Ron) but most of 
the documentation from Ron's patch remains. I've also updated the 
documentation to show how to create an anonymous sub to call a coderef 
with extra parameters.
> Better?
Yeah, if all tests pass for everyone else, let's release.

-Matt

Re: [Cgi-session-user] proposed refinements to load() and find() interfaces

[Ron S. <ro...@sa...>]

On Sat, 10 Jun 2006 16:11:38 -0500, Matt LeBlanc wrote:

Hi Matt

> Ok then. find is now changed back to three args (sorry Ron) but
> most of the documentation from Ron's patch remains. I've also
> updated the documentation to show how to create an anonymous sub to
> call a coderef with extra parameters.

It'll be fine with me as long as there is /some sort/ of mechanism whereby 
parameters can be passed to the find and then to the callback, and that's=
 what 
appears to be the case, so release and be damned!

-- 
Cheers
Ron Savage, ro...@sa... on 11/06/2006
http://savage.net.au/index.html
Let the record show: Microsoft is not an Australian company

Re: [Cgi-session-user] proposed refinements to load() and find() interfaces

[Mark S. <ma...@su...>]

On Sat, Jun 10, 2006 at 01:47:18PM -0500, Matt LeBlanc wrote:
> On 6/10/06, *Mark Stosberg* <ma...@su...> wrote:
> 
>    "load" vs "_load" is not clear.
> 
> load is part of the documented public interface, _load is not. This is a 
> change to internals, nothing else.

I understand. Still, having two names that are nearly identical can be a
sign that something is amiss, and is confusing for those who maintain
the code.

In this case, this difference is basically whether the ATIME gets
updated. 

I'm suggesting keeping one load() routine, and adding a "update_atime"
option, which we could possibly leave undocumented, since as far as we
are aware, it will only be turned off in one place interally, by find(). 

> The point of the change that I'm making is to allow us to inspect the 
> session as the user has left it without changing anything. I strongly 
> feel that the current behavior of find is wrong. find is supposed to 
> manage the sessions as the user has left them.

I like the change to not have find() modify ATIME. 

> I wouldn't mind coding them if I agreed with them =)

Fair enough. I don't mind leaving out the update_atime option to find, 
since you feel strongly about that. 

Here's another refactor:

1. Leave things generally alone with load(), but add extra param,
  for internal use only, just be clearer than load/_load

  load($dsn,$query, \%dsn_args, $update_atime);

2. Leave find() mostly alone as well, and don't add update_atime flag.
   Back out the coderef_args option patch in favor of documenting how
   to use a higher order function for this. 

  find( $dsn,   
		inspect( look => 'closely'), # will return a code ref
        \%dsn_args );

Better?

	Mark