Stop Fighting About Getters and Setters

Part of tinkering with SNES music development has been brushing up on C++, a language I haven't spent any meaningful time with since college, and it turns out that things have changed in the C++ world since 2008, largely for the better, as far as I can tell (hurray for auto). But as I've been skimming StackOverflow posts on what the hell constexpr is, when to use inline, why it's good practice for every line of code to contain the keyword const no fewer than thirty times, and so on, I've encountered a lot of people fighting over the use of getters and setters (accessors and mutators, for the pendantic).

So let's talk about that.

I'd ask how this isn't a solved issue, but I fairly clearly remember college instructors giving horrid advice like "all of a class's member variables should be private and have their own get and set functions." God forbid they learn about Python.

Following such advice might produce this wonderfully bloated code:

class Point {
public:
  float get_x() const { return _x; }
  void set_x(float x) { _x = x; } 
  float get_y() const { return _y; }
  void set_y(float y) { _y = y; }
private:
  float _x;
  float _y;
};

...which is, of course, a more-annoying-to-use-but-functionally-identical variant of the following:

class Point {
public:
  float x;
  float y;
};

But then you have these weirdly dogmatic developers wriggling out of the woodwork on SO to claim that any use of getters and setters means you're a bad developer who's organized his abstractions in a terrible manner. This is also a horrid take, otherwise the following function would be indicative of bad design:

size_t std::string::length() const;

It is, of course, not, because my_string.length = 140; (I know it's contrived; bear with me) would be a meaningless and potentially dangerous assignment. So let's just lay out two concrete rules for how getters and setters should work.

Rule one: use restrictive access modifiers, getters, and setters to protect the internal mechanisms of libraries you'll be distributing to other developers.

This should be pretty straightforward. Let's say I'm making a simple library for reading and playing SNES SPC music files. Part of what it will do, obviously, is load a file. When it does that, it's going to parse a bunch of meta information from the file and then populate some members that emulate SNES audio processing unit and digital signal processor RAM. I don't want people messing with those, because then the file won't play properly (if at all), so I'm going to lock them down.

A bigger takeaway from this rule is that if you're writing types that only you will ever use (i.e., you're writing a standalone piece of software, no piece of which will ever be distributed as a library), there's really no need to ever use restrictive access modifiers and getters. The only thing it would accomplish is protecting yourself from intentional self-sabotage -- a meaningless endeavor, given that there's no way to protect yourself from dragging all your code files to the trash can in the first place.

Rule two: use restrictive access modifiers, getters, and setters when they (especially setters) do more than what assignment to a public member would do, or to simplify/hide otherwise complex operations.

This is especially important in code you'll be distributing as a library, but can be useful for personal code, as well, as a helper. Let's say I have some kind of button component whose text can change in various circumstances, and every time I change its text member, I also want to redraw it. Instead of making developers (or myself!) write...

my_button.text = "Some new text";
my_button.redraw();

...over and over and over, I can make text private (for something I'm distributing) and provide this setter:

void set_text(const std::string& text) {
  _text = text;
  redraw();
}

That's a good, helpful use of a setter.

To revisit the SNES music example, let's say I want to give other developers the ability to skip to a specific millisecond in the track using a track_position member. That's a complex operation. The SNES APU can't tell me how many milliseconds deep into a track I am. I just feed it buffers of samples, and it plays them. But I know how quickly it plays them (32,000 times per second), so I can do the math to figure that out based on tracking my own buffering. I'm not going to make other developers do the math, and I'm certainly not going to give other developers access to the internal buffering mechanisms; that'd be a nightmare. But track_position can't just be a public member, because player.track_position = 10000; is meaningless.

So I'd probably write something like this (highly contrived, does not take lots of different things, like resampling, into account):

class Player {
public:
  int get_track_position() const { return _track_position; }

  void set_track_position(int position) {
    _clear_sample_buffers();
    _samples_read = (int)floor((position / 1000.0f) * 32000);
  }

  void play() { /* This will keep _track_position set accurately. */ }

private:
  SnesMusicFile* _music_file;

  int _track_position;
  int _samples_read;

  void _clear_sample_buffers(); 
};

That's it. Those are the two rules. And remember that, after everything, good practices are only as useful as other developers let them be. Visual Studio's intellisense will show you what the private members of types are, and in a language like C#, it's only a minor annoyance to reflectively access/modify them directly. In a language like Python, you can just directly modify whatever the hell you want. Developers always have the rope they need to hang themselves, if they really want to make use of it.

Do I think this is a good argument against ever using these techniques? Absolutely not. The people who make that claim are being foolish. The presence of restrictive access modifiers, getters, and setters should be an indicator to other developers how your code is intended to be used. It should let them know that you've provided a means of using your code clearly, reliably, and safely.