in reply to accessor abuse..?

Using a single method as both accessor and mutator isn't always bad, however this particular usage is because it probably isn't a documented behavior, and because the caller is mucking with "private" data in a way that violates the whole purpose of getters and setters (e.g., encapsulation)

If you want to use a single method for both purposes, it's better to do something like: 

sub foo {
  my ($this, $arg) = @_;
  $this->{foo} = $arg if defined $arg;
  return $this->{foo};
}

[download]

As to the last point, accessors do serve to protect values, but in Perl unless you do some extra work you're really just depending upon the politeness of your caller to respect your boundaries. For serious encapsulation take a look at closures (Why are closures cool?, Why are closures cool, continued?).

Update: as per tantarbobus's correction.

Replies are listed 'Best First'.
Re: Re: accessor abuse..?
by tantarbobus (Hermit) on Aug 22, 2002 at 15:06 UTC

    Don't you to return $this->{foo} and not $this->foo.

    Also, foo() breaks if you try to undefine $this->{foo} as in $this->foo(undef); so: 

    $test->foo("bob");
print "Bob:".$test->foo();
$test->foo(undef);
print "Undef:".$test->foo();

    [download]
    Will give: 
    Bob:bob
Undef:
    Instead of checking for defined() maybe look @ the number of args that are passed into foo, like such: 
    sub foo {
  my ($this, $arg) = @_;
  $this->{foo} = $arg if @_>1;
  return $this->{foo};
}

    [download]
[reply]
[d/l]
[select]

      Well, depends on how you want to go about signifying a 'null' value. Recall that an empty string is both defined and a common way of indicating that variable holds no useable value. So, calling the method like foo('') will effectively unset the variable. Of course in more complex situations (i.e., where the value to be set is an object reference, not just a string) you'd need something better, probably something along the lines of your suggestion to pass undef as a parameter.

[reply]
[d/l]

        I agree with you about using an empty string, but if you are going to be ignoring the undef case, then it probably should be documented because if someone, like me, were to wonder upon that bit of code, s/he might be tempted to see the behaviour as a bug and not a feature (after all it did not do what I told it to when I spoke $self->foo(undef) :) So maybe: 

        ##########
# foo()
# Sets or returns the value of foo.
# But will keep the old value of foo
# if undef be passed in

sub foo{ 
    # Do foo stuff
}

        [download]

        Another option would be to use a different function name that reflects the no-blow-away-existing-values-on-undef behaviour.

[reply]
[d/l]
Re^2: accessor abuse..?
by Aristotle (Chancellor) on Aug 22, 2002 at 23:23 UTC
    I would opt for 
    sub foo {
    my $self = shift;
    return @_ ? $_ = shift : $_ for $self->{_foo};
}

    [download]

    Makeshifts last the longest.

[reply]
[d/l]

In Section Seekers of Perl Wisdom