http://qs321.pair.com?node_id=189073

aufrank has asked for the wisdom of the Perl Monks concerning the following question:

hey all--

I'm working on a method in my first OO project which seems like it should be simple but ended up pretty complex (and still doesn't DWIW). The idea is that I only want to allow certain values to be passed as parameters, and I want to assign a default if the value passed is not one of the legal ones. Here my attempted WTDI:

#! perl -w use strict; package Test; sub new { my $class = $_[0]; my $self = {'associations' => undef, 'balance_by' => undef, 'comment' => undef}; bless $self, $class; return $self; } sub balance_by { my $self = shift; my $entry = shift; my %valid_params = map {$_ => 1} ('foo', 'bar'); unless (defined $self->{'balance_by'}) { $self->{'balance_by'} = (defined $entry && $valid_params{$entry}) ? $entry : 'foo'; } return $self->{'balance_by'}; } my $lex = Test->new(); $lex->balance_by('bar'); print $lex->balance_by();

initially this seemed to work fine, but when I went back and added the line

print $lex->balance_by('baz')
it still prints "bar". I realize now that the way I used the unless loop makes it impossible to update the value once it has been defined once, but I am at a loss. How should I go about doing this?

thanks in advance,
--au

this was going to be a question about lists, arrays, $_, @_, and shift, but an excellent CB discussion unmuddied my head a bit (or at least showed me that the subject is inherently muddy ;-)

Replies are listed 'Best First'.
Re: only allow certain parameters
by Ovid (Cardinal) on Aug 09, 2002 at 22:45 UTC

    Right off the bat, this is what I thought of:

    sub balance_by { my ($self,$entry) = @_; my %valid_params = map {$_ => 1} qw/ foo bar /; $self->{'balance_by'} = (defined $entry and exists $valid_params{$ +entry}) ? $entry : 'foo'; return $self->{'balance_by'}; }

    The only real change is the removal of the unless logic. What that does is set the "balance_by" if you have a valid $entry>, else it sets it to the default "foo". However, it seems to me that you wanted a one-time setting of balance_by that could not be changed. Is that correct? In your code "foo" will only be used as a default on the first pass.

    So, if I understood what you wanted, my code will always set balance_by to $entry if you have a valid $entry. If it's not valid, it sets it to "foo".

    Cheers,
    Ovid

    Join the Perlmonks Setiathome Group or just click on the the link and check out our stats.

Re: only allow certain parameters
by runrig (Abbot) on Aug 09, 2002 at 22:49 UTC
    I assume you want to set the value if a parameter is passed or if the current value is undef, and just return the current value otherwise:
    sub balance_by { my $self = shift; my %valid_params = map {$_ => 1} ('foo', 'bar'); if (@_ or ! defined $self->{'balance_by'}) { my $entry = shift; $self->{'balance_by'} = (defined $entry and $valid_params{$entry}) ? $entry : 'foo'; } return $self->{'balance_by'}; }
    I would also define your %valid_params outside of the subroutine. No need to define them each time you call the sub. Also if you want to nano-optimize your code, consider using exists to see if the key exists in valid_params instead of testing its value, like so:
    my %valid_params; undef @valid_params{qw(foo bar)}; ... (defined $entry and exists $valid_params{$entry}) ...
    Also, unless I'm mistaken, there doesn't seem to be any reason to not default your fields in the 'new' method...(unless you just wanted 'if (@_)...' in that conditional?)
Re: only allow certain parameters
by dws (Chancellor) on Aug 09, 2002 at 23:09 UTC
    The idea is that I only want to allow certain values to be passed as parameters, and I want to assign a default if the value passed is not one of the legal ones.

    This approach hides problems. If you're going to do this, at least log failures so that they can be tracked down and resolved, rather than leaving clients scratching their heads of mysterious behavior.