Reading through some old posts I've run into multiple mentions of the idea that "accessors break encapsulation". (See here, for example.) I think this idea has some merit for languages like C++ or Java in which one can assign levels of privacy to methods, and thereby enforce one's published API, although it would have to be rephrased as "public accessors break encapsulation".
But with Perl it's not so easy to enforce a distinction between the published API and a class's private methods. For the vast majority of published classes I've looked at, "private" methods are simply those that are not explicitly described in the official documentation. If someone finds out what these unpublished methods are, e.g. by reading the source code, he/she can use them. (Of course, this is a bad idea, but my point is that, in this common situation, Perl provides no encapsulation, other than that granted through "the kindness of strangers".)
I find accessors are useful internally, for the same reason they are useful externally: they provide insulation against whatever changes may affect the implementation of a state variable. Therefore, even when I don't publish most accessors, I like to have plenty of them (I usually give these "private" accessors names that begin with _, FWIW).
Of course, I could do the "lexical accessors" trick (i.e. implement accessors as coderefs stored in lexical variables), but this is problematic in two ways. One is that subclasses cannot inherit these accessors, which, more often than not is a PITA. And two, I can't directly test these "lexical accessors" in my test suite.
This last point brings up another important question, entirely orthogonal to the accessors issue: should a test suite test only the published API, or should it also test private methods? In my view, the utility of a test suite for refactoring is greatly diminished if it doesn't test private methods... I suppose that a module could include some internal tests that can have access to module-private lexicals, but I digress.
As always, I look forward to reading your comments.
Update: Added a "?" after the title. Added link to Hollub's article.
|
Re: "Accessors (don't always) break encapsulation"
by Ovid (Cardinal) on Jul 19, 2005 at 17:17 UTC
|
Whether or not an accessor violates encapsulation depends upon the class. While there's a good argument that you should often just send messages to an object and let it decide how to respond rather than pull bits of data out of it, that's for objects that I see as modelling a process or need. A perfect example is this oft-repeated mistake:
if ($obj->error) {
$obj->log_error;
}
With code like that, the programmer has to remember to check for an error every time she might need to log one. More stuff to remember means more bugs. Instead, push the $obj->error call into the &log_error method and let the object ignore the message if there are no errors (though even exposing the &log_error method may not be good.)
However, sometimes a class can merely be viewed as a data type where you have fine-grained control over the domain of values the data can represent. I mean, I could create an integer variable which is only supposed to hold prime numbers. I could also create a Prime class which does the same thing but actually restricts me to prime numbers. In the latter case, my code may arguably be more correct but there's also nothing wrong with asking what the prime number is.
As for whether or not you should only test your published API, people disagree about that, also. On the "purity" side, there are those who argue that only the published API should be tested as that's all you are promising. Plus, testing the internals means you're more likely to break tests as you refactor, add, or delete code. However, I do like to test the internals, though I will often keeps those tests separate. The reason I do this is the API is often like the tip of an iceberg. There's a whole lotta code underneath. While you definitely want to test the API (and probably test that first), if you have a lot of code that this doesn't test directly, you may find a bug but have a much harder time tracking it down. Internals testing means you quickly find what's really broken, though this can mean more maintenance down the road.
| [reply]
[d/l] |
|
|
if ($obj->error) {
$obj->log_error;
}
I assume that you are referring to some of the logging modules out there that do this (eg, Log::Log4perl). In the simple case I agree with you (and it's actually what most of them do behind the scenes anyway). But the problem occurs when you have a computationally expensive error string. For instance:
use Data::Dumper;
$obj->log_error(Dumper($some_very_large_structure));
By using the first approach, this expensive call could be avoided if the object just gave you an accessor so that you could make the decision yourself if the logging was necessary.
-- More people are killed every year by pigs than by sharks, which shows you how good we are at evaluating risk. -- Bruce Schneier
| [reply]
[d/l]
[select] |
|
|
Actually, I wasn't referring to those modules at all. Ignoring potential issue with premature optimization -- issues that I feel are more significant in Perl as it's rather slow and we don't have lazy evaluation -- if a logging module is there to allow the user to log stuff, then of course we need to figure out whether or not it's appropriate to call the method. In the example I used, it was a hypothetical example regarding something the programmer shouldn't have to worry about. In your example, you present an external API that the programmer should decide whether or not to call because that's the entire rationale behind the module.
Sorry if I wasn't clear.
| [reply] |
|
|
|
|
I encountered almost exactly this situation. Here was my solution to keep code simple and avoid the excessive hit.
$obj->log_on_error{sub {Dumper($some_very_large_structure)});
The standard logging modules may not provide this functionality though. | [reply]
[d/l] |
Re: "Accessors break encapsulation"
by hardburn (Abbot) on Jul 19, 2005 at 14:43 UTC
|
Yes, the general rule really only applies to public accessors and mutators. From the outside, you should be telling an object what to do and then let the object change its own state.
should a test suite test only the published API, or should it also test private methods?
A test suite should aim for the highest practical code coverage. For every public method called, there are probably some underlieing private methods called. The test suite may then end up having complete coverage of the object without actually calling private methods.
"There is no shame in being self-taught, only in not trying to learn in the first place." -- Atrus, Myst: The Book of D'ni.
| [reply] |
|
|
| [reply] |
|
|
| [reply] |
|
|
|
|
|
|
|
Re: "Accessors break encapsulation"?
by pg (Canon) on Jul 19, 2005 at 23:59 UTC
|
It is the practice not the concept breaks encapsulation.
As a concept, getter/setter does not break one thing. Your object needs some way to communicate and interact with its surrounding, doesn't matter whether you call that communication channel getter/setter or mouth/ear, you cannot avoid it.
If the programmer provides unsafe access to an object through getter/setter, it is the programmer's problem, because the programmer abuses the concept and doesn't use it wisely.
If we are only talking about Perl, then seems to me the real problem is that, on one hand, Perl tries to provide you a way to nicely packaging your modules, on the other hand, it is the nature of the language that provides ways to break what it tries to protect, however you can also call that freedom, just like any other such things in one's daily life.
| [reply] |
|
|
| [reply] |
Re: "Accessors break encapsulation"?
by fergal (Chaplain) on Jul 19, 2005 at 23:25 UTC
|
There's nothing in Hollub's article that doesn't also apply to public member variables. His getX example applies equally to a member variable X. The real point is that providing access to implementation dependent parts of objects is bad and providing it through an accessor doesn't magically make it OK.
In C++, Java, Perl, people rightly add accessors to prevent reliance on direct access to member variable because if all the clients treat something as a member variable then you can't change that without breaking the clients. So instead of public members you end up with private members + set/get methods.
The problem is now that all your members are private - both the really private stuff and the stuff that's intended for public use. This is where confusion begins. The "privacy" of a member variable is now decided by it's accessor and the declarations that goes along with the actual member are redundant. Word then gets around that the "correct" way to program is to make all your member variables private and write accessors for them. Then there are tools which will do this for you but alas there is no longer any way to indicate which members should be private and which public (they're all marked private). Next thing you know, there are public accessors created for all member variables so now everything is effectively public.
Object Pascal (a frustrating language) comes pretty close to getting this right. It allows you to declare things which look like member variables and can be public, private etc but when you read it it calls a getter and when you set it it calls a setter. The client neither knows nor cares what's happening in the background and you get to keep meaningful public/private declarations.
Basically don't expose it if it's not part of the external spec of the object. Some objects are supposed to have "data slots", some aren't. Don't go creating data slots when you shouldn't.
As for testing, test whatever it is useful to test. Forget dogma. If you don't test a private function you lose all the benefits of testing. If you do test it, the worst that can happen is that when you refactor, the test will become irrelevant or wrong. So what? This could also happen to your publised API tests if you decide to change the API.
That said, it might be a good idea to separate the tests. Have one file for the public API and one file to test the inner workings.
| [reply] |
|
|
