An interesting topic came up in #phpc today. It revolved around some issues I’ve been encountering in my latest code designs/structures. It also leads into some side topics that I will attempt to explore.
From my point of view, the discussion centered around what is the best ‘default’ visibility to use for methods. Another thing touched upon is the Open/Closed principle, which I think I subscribe to or may subscribe to(this depends on my ability to determine what half of the words in the entry mean).
Out of the whole discussion, here is the points I got (aka understood) out of it. Please correct me in the comments if I’m off base in any way.
Methods should only be public when necessary. This is to help reduce the amount of side-effects that can occur because of method overrides.
K, I can accept that and in thinking about my past code, I use to use protected more than private. I used public very sparingly.
Now I find, since I started unit testing, that I have a large amount of public functions than I’ve had in the past. That is because I can’t figure out how to test private methods. One way that I can think of is to create public methods that allow you to test the private ones. Unfortunately, this makes me wonder why the method isn’t public to begin with since it seems redundant and wasteful to have these public methods to access private methods.
Please note the key word methods, property accessors are a different story.
The best solution (I can think of) to testing private methods is Mock Objects. Unfortunately, even though I’ve started to use mocks/doubles more, I’m unsure if my implementation is correct. Until I’m confident in my understanding of mocks/doubles, I worry about having false positives in test results.
Another assertion that was made in the discussion was that private methods allow you to preserve the class’ core functionality.
Unfortunately, no matter what the visibility of the method is, you’re able to override it (and potentially mess with the core functionality you were trying to preserve).
Example:
class foo{
private function foobar() {
echo "foo\n";
return "foo foo\n";
}
public function bar() {
echo "w00t ";
return $this->foobar();
}
}
class bar extends foo{
private function foobar() {
return "bar\n";
}
public function baz() {
echo $this->bar();
}
public function wut() {
echo $this->foobar();
}
}
$f=new bar();
$f->baz();
$f->wut();
By running the above code, you get:
w00t foo foo foo bar
If preservation of core functionality is your main concern, then you’re better off using final.
So after all that rambling, you’re probably wondering what I’m trying to get to. It still comes down to visibility’s effect on inheritance. The way I see it, unless you declare the method as final, you can’t lock down the parent functionality because private methods can still be overridden. Trying to figure out what level of visibility for a method is a situational call. There is no correct ‘default’ visibility. Sure private is safer because of least privilege but it makes testing a bear (or at least a bear at my current skill level in testing). Public potentially opens you up for abuse or misuse.
What I would still like to know is, how do you do class method visibility? How does that affect your testing methodology? Can someone give me an example (that isn’t a singleton) where private is a better choice over protected? I like protected because it seems less limiting to me and my current coding style appreciates that degree of flexibility.
@ingorenner pointed out to me on twitter that “It’s more important to assure that the public API is working. How internals work – thus private – is up to you”
And he also pointed out that I had lame register and login to comment on. I’ll have to look into captcha for spam filtering but think my akismet should get it. Guess I’ll find out. Anyways, the thing I have with that statement is mostly likely due to my nubbish skills as a programmer and developer. So let me explain part of how I’ve been developing with tests.
If a public function I have contains a private function. Then while test/debugging, how do make sure that the private function or functions aren’t part of a logic error that I’m trying to debug.
The root of my issue with private functions and testing may be due to the fact that some of my function do contain more than 1 private function that may be the cause of a test failing. Having a test for each function called within another function has been a benefit of unit testing for me.
You shouldn’t really think about implementation – the non-public bits – while you’re writing tests, just interfaces and behaviours. Think about what the client expects to happen when a button is pressed or a lever is pulled and describe that in a test. Keep adding new fixtures (and tests) until you’ve covered everything you need to cover. Any private/protected bits which don’t get exercised by all can be deleted: they’re not required. It helps if you write tests first and then code.
Mocks are intended to expose internal workings but only so you can “discover” the neighbouring objects in the design. TDD is primarily about design rather than testing – although that too.
Personally, I never use private or protected. They don’t actually do anything useful. http://aperiplus.sourceforge.net/visibility.php
> …then you’re better off using final.
Exactly but you have got to be careful and not just plaster it all over the place.
The way I look at it is “public”, “private”, and “protected” provide an aid for documentation and a guide to the unwary developer as to the intended wishes of the original developer(s).
And who knows? A future version of PHP may actually fix any current issues and we do have the expected behaviour.
> Personally, I never use private or…
Personally I wish you would stop plugging your silly little testing library all over the web.
I agree TDD is important but there are already far better, more mature tools out there that do the same job… but only better.
Be gone with you McGuff.
I think there must be some kind of misunderstanding. I didn’t intend to post spam to your blog. There is a link to a piece about visibility in php which I thought might be relevant to your question (private is evil) but I don’t actually have a rival unit test library to promote. I have written some extensions for simpletest – which I didn’t actually mention in my post – and they are available on sourceforge but I don’t really “plug” them because they’re too tuned in to my own specific needs (for example no windows support at present). They’re there, and the rest of the library, to share ideas and to promote testing.
I like to help people learn about testing, if I can, and I tried to help you. I don’t understand why you should feel offended.
When i started coding php, one of the big gestatlt moments I had is when i realized how much time I was saving by NOT having to think about private/protected/public methods in my applications – and how much time I had wasted before for no gain at all. That was php4 time of course, and all methods where public. Just use an underscore for methods ‘meant’ to be private, and you can have a nice api with private separate from public and do unit tests at the same time. Enforcing ‘private’ at the compiler level gains you very little, as if you have a coder in your team that breaks the convention he’s either a moron that’d benefit of some training or a guru who’s more effective at breaking standards than you would be at following them.
This of course applies to applications, not frameworks, where the code will be reused for ages in different contexts, and it benefits of course of the php execution model of wipe-everything-at-end-of-page…
Public, public for all!
I take the view of if you are creating code, you are writing a test for it, and you make the test pass with the barest of implementations.
You come back the next day, and realise you are repeating the same pattern of checking input in a number of places – ie, a validate().
This was a hidden design problem, so you decide to implement a protected method and shift the common code up.
However, when you really think about it, shifting a method up because of a hidden design problem really means you haven’t modelled it properly; and your only real option from here is to make it another class.
That way you can break the responsibility from your first object into another object, and increase the specialisation of each.
The payoff for creating a whole new class might be minimal if all it does is validate() though; so usually I apply my Best Judgement, and simple expose the method as public / write test coverage of it, with a @todo to refactor it elsewhere or add it to an interface if it seems common enough.
Doing a Bad Thing once is unpleasant, twice is ugly, three times is a good time to refactor.
I choose to use protected methods by default, and go to public only for methods that I implement to follow the interface I spec’d out. In choosing how I architect my interface, I’m choosing how I’ve modeled it to be USED. How it does its own work, that’s in the protected methods.
When it comes to testing, I always prepare a mock object for each class, at least so that future tests that need a mock of the given class will have one in place that was built while the new class was still fresh in my mind. Its mocked public methods are designed to return controlled responses, just like you’d want from a good mock object. In addition, I’ll create public wrapper methods that wrap around the original class’s protected methods, thereby making each protected method testable by itself. This isn’t about code coverage or the API… it’s about me being more thorough in guaranteeing the behavior of my protected methods.
By segregating logic into small units that each “do one thing and do it well”, I typically end up with many protected methods. By trying to keep my API as clean as I can, and true to my architectural intent, I limit the amount of public methods. Tests against my public methods still exercise the protected methods that they use, true enough. My insistence on the mocked object’s public wrappers allowing me to directly test the protected methods is just additional insurance, done in a way that I can more closely target each protected method’s behavior.
I’ll rarely if ever choose private for anything. Nearly all code I write is done with the expectation that it is or could be reused elsewhere… as such, I trust to protected methods for allowing child classes to do what they need to do, whether it’s decorating the parent’s protected methods or overriding them completely.
I was actually *very* surprised to see your example of a private parent method being overridden by a child class. Did the #phpc discussion touch on this being a visibility enforcement bug?
Looking at the documentation for visibility in PHP it seems the point is exactly visibility – i.e. having virtual functions in parent and child class means that both of them can safely call the method + you can change behaviour as needed.
Apart from that, I stick to using protected functions mainly, only exposing the functions that should be called outside a class. One of the main reasons for this is the experience from working in an international organisation on a volunteer basis – everyone comes from a different background, with different experiences, and there’s a high chance that you will see some code used in ways you hadn’t quite planned. If this leads to tight coupling (and it almost certainly will), then you’re headed down the nightmare path.
The more public methods a class has, the more trouble you’re looking at in terms of maintenance when you have to update the class or switch it out.
Regards
Fake