| Subject: | notabug: some minor cleanup in sub param() |
In the attached patch, I suggest a few changes in param() that
(1) help the flow control expressions stand out
(2) get rid of a C-style for loop
(3) rename $ok as $is_modified
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-param-4.14.patch |
--- Session-original.pm 2006-06-11 15:13:35.000000000 +0300
+++ Session-edited.pm 2006-09-29 23:17:48.000000000 +0300
@@ -229,66 +229,74 @@
sub tracemsg {}
sub param {
- my $self = shift;
+ my ($self, @args) = @_;
- carp "param(): attempt to read/write deleted session" if $self->_test_status(STATUS_DELETED);
+ if ($self->_test_status( STATUS_DELETED )) {
+ carp "param(): attempt to read/write deleted session";
+ }
- #
- # USAGE: $s->param();
- # DESC: returns all the **public** parameters
- unless ( @_ ) {
+ if (@args == 0) {
+ #
+ # USAGE: $s->param();
+ # DESC: Returns all the _public_ parameters
+ #
return grep { !/^_SESSION_/ } keys %{ $self->{_DATA} };
}
-
- #
- # USAGE: $s->param($p);
- # DESC: returns a specific session parameter
- return $self->{_DATA}->{$_[0]} if @_ == 1;
-
- my %args = (
- -name => undef,
- -value => undef,
- @_
- );
-
- #
- # USAGE: $s->param(-name=>$n, -value=>$v);
- # DESC: updates session data using CGI.pm's 'named parameter' syntax. Only
- # public records can be set!
- if ( defined( $args{'-name'} ) && defined( $args{'-value'} ) ) {
- if ( $args{'-name'} =~ m/^_SESSION_/ ) {
+ elsif (@args == 1) {
+ #
+ # USAGE: $s->param( $p );
+ # DESC: returns a specific session parameter
+ #
+ return $self->{_DATA}->{ $args[0] }
+ }
+
+ my %args = @args;
+ my ($name, $value) = @args{ qw(-name -value) };
+ if (defined $name && defined $value) {
+ #
+ # USAGE: $s->param( -name => $n, -value => $v );
+ # DESC: Updates session data using CGI.pm's 'named param' syntax.
+ # Only public records can be set!
+ #
+ if ($name =~ m/^_SESSION_/) {
carp "param(): attempt to write to private parameter";
- return undef;
+ return;
}
- $self->_set_status(STATUS_MODIFIED);
- return $self->{_DATA}->{ $args{'-name'} } = $args{'-value'};
+ $self->_set_status( STATUS_MODIFIED );
+ return $self->{_DATA}->{ $name } = $value;
}
- #
- # USAGE: $s->param(-name=>$n);
- # DESC: access to session data (public & private) using CGI.pm's 'named parameter' syntax.
- return $self->{_DATA}->{ $args{'-name'} } if defined $args{'-name'};
-
- # USAGE: $s->param($name, $value);
- # USAGE: $s->param($name1 => $value1, $name2 => $value2 [,...]);
- # DESC: updates one or more **public** records using simple syntax
- unless ( @_ % 2 ) {
- my $ok;
- for ( my $i=0; $i < @_; $i += 2 ) {
- if ( $_[$i] =~ m/^_SESSION_/) {
+ if (defined $name) {
+ #
+ # USAGE: $s->param( -name => $n );
+ # DESC: Access to session data (public & private) using
+ # CGI.pm's 'named param' syntax.
+ #
+ return $self->{_DATA}->{ $name };
+ }
+ if (@args % 2 == 0) {
+ #
+ # USAGE: $s->param( $name, $value );
+ # USAGE: $s->param( $name1 => $value1, $name2 => $value2 [,...] );
+ # DESC: Updates one or more _public_ records using simple syntax
+ #
+ my $is_modified = 0;
+ ARG_PAIR:
+ while (($name, $value) = each(%args)) {
+ if ($name =~ m/^_SESSION_/) {
carp "param(): attempt to write to private parameter";
- next;
+ next ARG_PAIR;
}
- $self->{_DATA}->{ $_[$i] } = $_[$i+1];
- $ok++;
+ $self->{_DATA}->{ $name } = $value;
+ ++$is_modified;
}
- $self->_set_status(STATUS_MODIFIED);
- return $_[1] if @_ == 2 && $ok;
+ $self->_set_status( STATUS_MODIFIED );
+ return $args[1] if @args == 2 && $is_modified;
return 1;
}
- #
- # If we reached this far none of the expected syntax were detected. Syntax error
+ # If we reached this far none of the expected syntax were
+ # detected. Syntax error
croak "param(): usage error. Invalid number";
}