Skip Menu |

This queue is for tickets about the CGI-Session CPAN distribution.

Report information
The Basics
Id: 21785
Status: rejected
Priority: 0/
Queue: CGI-Session

People
Owner: Nobody in particular
Requestors: fenlisesi [...] gmail.com
Cc:
AdminCc:

Bug Information
Severity: Wishlist
Broken in: 4.14
Fixed in: (no value)



Subject: notabug: generator for subs id(), atime() and ctime()
In the attached patch, I suggest a generator for subs id(), atime() and ctime(), which are accessors to readonly session attributes. I like the patch, but it may do more harm than good -- your call. If you like it too, I could post a similar one for is_expired() and is_new(). The patch is against 4-14. It passes all the non-skipped tests that come with the 4-14 distribution on my machine, but please examine it closely anyway, as nobody reviewed it. $CGI::Session::VERSION = '4.14'; This is perl, v5.8.8 built for i686-linux-thread-multi Linux version 2.6.11.4-21.14-default (geeko@buildhost) (gcc version 3.3.5 20050117 (prerelease) (SUSE Linux)) #1 Thu Aug 24 09:51:41 UTC 2006
Subject: CGI-Session-id-atime-ctime-4.14.patch
--- Session-original.pm 2006-06-11 15:13:35.000000000 +0300 +++ Session-edited.pm 2006-09-30 03:10:45.000000000 +0300 @@ -77,11 +77,25 @@ sub is_new { $_[0]->_test_status( STATUS_NEW ) } -sub id { return defined($_[0]->dataref) ? $_[0]->dataref->{_SESSION_ID} : undef } - -sub atime { return defined($_[0]->dataref) ? $_[0]->dataref->{_SESSION_ATIME} : undef } - -sub ctime { return defined($_[0]->dataref) ? $_[0]->dataref->{_SESSION_CTIME} : undef } +## Non-inheritable private sub. Generates closures for session attr access +sub _gen_session_attr_accessor { + my ($attr) = @_; + my $attr_access_key = '_SESSION_' . uc( $attr ); + return sub { + my ($self) = @_; + my $dataref = $self->dataref(); + if (defined $dataref) { + return $dataref->{ $attr_access_key }; + } + return; + }; +} +BEGIN { + no strict 'refs'; + foreach my $attr (qw(id atime ctime)) { + *{ $attr } = _gen_session_attr_accessor( $attr ); + } +} sub _driver { my $self = shift;
On Fri Sep 29 20:25:02 2006, fenlisesi@gmail.com wrote: Show quoted text
> In the attached patch, I suggest a generator for subs > id(), atime() and ctime(), which are accessors to > readonly session attributes. I like the patch, but > it may do more harm than good -- your call. If you > like it too, I could post a similar one for is_expired() > and is_new(). > > The patch is against 4-14. It passes all the non-skipped > tests that come with the 4-14 distribution on my machine, > but please examine it closely anyway, as nobody reviewed it.
Thanks for this submission. I see that it removes some repetition from the code, along the lines of the DRY principle. However, I'm rejecting it because it makes the code less clear, with no apparent gain. Mark