Question

I've seen some APIs, especially in scripting languages (we use Perl and JS in our team), that use combined getter and setter methods. For example, in jQuery:

//append something to element text
var element = $('div#foo');
element.text(element.text() + 'abc');

For example, in Perl's CGI.pm module:

# read URL parameter from request
my $old_value = $cgi->param('foo');
# change value of parameter in request
$cgi->param('foo', $new_value);

Some generic DAO class in our Perl codebase uses a similar pattern. Autogenerated accessors call into an internal getset() method, similar to this:

sub foo { # read/write accessor for the foo() property (autogenerated)
    my ($self, $new_value) = @_;
    return $self->getset('foo', $new_value);
}

sub getset {
    my ($self, $field, $new_value) = @_;
    ## snip (omitted some magic here) ##

    # setter mode
    if (defined $new_value) {
        ## snip (omitted some magic here) ##
        $self->{data}{$field} = $new_value;
        ## snip (omitted more magic here) ##
    }

    # getter mode
    return $self->{data}{$field} // '';
}

I see multiple issues with this design:

  • Setter calls go through the getter code path, too, which is inefficient especially if you have relations, so the getter has to resolve the stored ID into an object:

    $foo->bar($new_bar); # retrieves a Bar instance which is not used
    
  • A getter call needs to carry the $new_value argument around. When you call a Perl method very often (say, 100000 times, a usual number of records in reporting and other cronjobs), this can already cause measureable overhead.

  • Setters cannot take an undefined value. (A Perl specific issue, which I could work around by checking for argument count instead of argument defined-ness, but this would make the autogenerated accessors even more complicated, and probably break some manual accessors.)

  • Decreased grepability: If I had separate getters and setters (e.g. foo() and set_foo() for each foo property), I could search for setter calls using a simple grep.

  • Even more?

I wonder if there are any actual benefits for the combined-setter-and-getter approach, or if it's just a weird tradition among the communities of the respective languages/libraries.

Was it helpful?

Solution

Synopsis

  • Having seperate getters/setters or having combined accessors is a cultural preference. There is nothing that prevents you from choosing the path less taken.
  • Most of your downsides don't exist. Do not confuse implementation details with problems of a stylistic decision.

Having different set_ and get_ methods looks self-documenting, but

  • are a pain to write if no autogeneration takes place, and
  • are primarily a form of static access control: I can have a get_ without exposing a set_.

The latter point is especially important where fine-grained access control and permission systems are used, e.g. in languages like Java, C#, C++ with visibility nuances like private/protected/public, etc. As these languages have method overloading, writing unified getters/setters isn't impossible, instead it is a cultural preference.

From my personal perspective, unified accessors have these benefits

  1. The API is smaller, which can make documentation and maintenance easier.
  2. Depending on the naming convention, there are 3–4 less keystrokes per method call.
  3. Objects feel a bit more struct-like.
  4. I can think of no real downsides.

It is my personal opinion that foo.bar(foo.bar() + 42) is a tad easier on the eyes than foo.setBar(foo.getBar() + 42). However, the latter example makes it clear what each method does. Unified accessors overload a method with different semantics. I consider this to feel natural, but it obviously complicates understanding a code snippet.

Now let me analyze your alleged downsides:

Analysis of alleged issues with combined getters/setters

Setter calls go through the getter code path

This isn't neccessarily the case, and rather a property of the implementation you are looking at. In Perl, you can reasonably write

sub accessor {
  my $self = shift;
  if (@_) {
    # setter mode
    return $self->{foo} = shift;
    # If you like chaining methods, you could also
    # return $self;
  } else {
    # getter mode
    return $self->{foo}
  }
}

The code paths are completely seperate, except for stuff that is truly common. Note that this will also accept an undef value in setter mode.

[…] retrieves a Bar instance which is not used

In Perl, you are free to inspect the context in which your method was called. You can have a code path that is executed in void context, i.e. when the return value is thrown away:

if (not defined wantarray) { be_lazy() }
A getter call needs to carry the $new_value argument around […] measureable overhead.

Again, this is an implementation-specific problem. Also, you overlooked the real performance issue here: all you accessor does is to dispatch a method call. And method calls are slow. Of course method resolution is cached, but this still implies one hash lookup. Which is so much more expensive than an extra variable on the argument list.

Note that the accessor could also have been written like

sub foo {
   my $self = shift;
   return $self->getset('foo', @_);
}

which gets rid of half of the I can't pass undef problem.

Setters cannot take an undefined value.

… which is wrong. I already covered that.

Grepability

I will use this definition of grepability:

If a source file is searched for the name of a method/variable/…, then the site of declaration can be found.

This forbids moronic autogeneration like

my @accessors = qw/foo bar/;
for my $field (@accessors) {
  make_getter("get_$field");
  make_setter("set_$field");
}

Here, set_foo wouldn't bring us to the point of declaration (the above snippet). However, autogeneration like

my @accessors = qw/foo bar/;

for my $field (@accessors) {
  make_accessor($field);
}

does fulfill the above definition of grepability.

We can use a stricter definition if we like:

If a source file is searched for the declaration syntax of a method/variable/…, then the site of declaration can be found. This would mean “function foo” for JS and “sub foo” for Perl.

This just requires that at the point of autogeneration, basic declaration syntax is put into a comment. E.g.

# AUTOGENERATE accessors for
# sub set_foo
# sub get_foo
# sub set_bar
# sub get_bar
my @accessors = qw/foo bar/;
...; # as above

The grepability is not impacted by using combined or seperate getters and setters, and only touches on autogeneration.

On non-moronic autogeneration

I don't know how you autogenerate you accessors, but the result that is produced doesn't have to suck. Either you use some form of preprocessor, which feels silly in dynamic languages, or you use eval, which feels dangerous in modern languages.

In Perl, I'd rather hack my way through the symbol table during compile time. Package namespaces are just hashes with names like %Foo:: that have so-called globs as entries, which can contain coderefs. We can access a glob like *Foo::foo (note the * sigil). So instead of doing

package Foo;
sub foo { ... }

I could also

BEGIN {
  *{Foo::foo} = sub { ... }
}

Now let's consider two details:

  1. I can turn off strict 'refs' and can then dynamically compose the subroutine name.
  2. That sub … it is a closure!

Therefore, I can loop over an array of field names, and assign them the same subroutine, with the difference that each closes over a different field name:

BEGIN {
  my @accessors = qw/foo bar/;
  # sub foo
  # sub bar

  for my $field (@accessors) {
    no strict 'refs';

    *{ __PACKAGE__ . '::' . $field } = sub {
      # this here is essentially the old `getset`

      my $self = shift;

      ## snip (omitted some magic here) ##

      if (@_) {
        # setter mode
        my $new_value = shift;
        ## snip (omitted some magic here) ##
        $self->{data}{$field} = $new_value;
        ## snip (omitted more magic here) ##
        return $new_value; # or something.
      }
      else {
        # getter mode
        return $self->{data}{$field};
      }
    };
  }
}

This is greppable, more efficient than just delegating to another method and can handle undef.

The downside to this is the reduced maintainability, if the maintaining programmer does not know this pattern.

Also, the errors originating inside that sub are reported as coming from __ANON__:

Some error at script.pl line 12
    Foo::__ANON__(1, 2, 3) called at Foo.pm line 123

If this is an issue (i.e. the accessor contains complex code), this could be mitigated by using Sub::Name, as Stefan Majewsky points out in the comment below.

OTHER TIPS

There are benefits, but they are outweighed by downsides.

In typical cases, readability, or ease of understanding is the most important concern for code.

I think developers of this pattern are trying to be concise, which is important. The more methods in your API, the harder it is to understand.

However, a well designed function should only do one thing. This trumps any benefit from being concise.

Another aspect to consider is whether the language supports lvalue accessors. For example, in Perl you can apply mutation operators to such accessors, arriving at things like

$obj->name =~ s/foo/bar/;
$obj->value += 10;

which without an lvalue accessor+mutator method, would require a much less neat

$obj->set_name( $obj->get_name =~ s/foo/bar/r );  # perl 5.14+
$obj->set_name( do { $_ = $obj->get_name; s/foo/bar/; $_ } );  # previously

$obj->set_value( $obj->get_value + 10 );
Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top