public or protected?
Moderator: General Moderators
- Ambush Commander
- DevNet Master
- Posts: 3698
- Joined: Mon Oct 25, 2004 9:29 pm
- Location: New Jersey, US
public or protected?
I am currently in the process of converting a PHP 4 codebase to PHP 5, this means fully specifying PPP (public/protected/private).
Most of the time, the code helpfully points out in the docblock what visibility I should assign to a method. However, I'm frequently running into cases where the code claims a method is protected, but it's invoked by the unit test in a TDD-esque style into order to be tested.
Ideally speaking, I should probably factor these out to a separate class and treat them as public. However, as of right now, I'm simply trying to get all the PPP's set so I can commit to the repository (and, also, so that the code doesn't throw noodles of E_STRICT errors when I run PHP 5.0). Should I simply give in and keep them as protected, or should I protect them and either 1. throw out their unit tests or 2. create a subclass that has a public interface for the protected method?
Thanks.
Most of the time, the code helpfully points out in the docblock what visibility I should assign to a method. However, I'm frequently running into cases where the code claims a method is protected, but it's invoked by the unit test in a TDD-esque style into order to be tested.
Ideally speaking, I should probably factor these out to a separate class and treat them as public. However, as of right now, I'm simply trying to get all the PPP's set so I can commit to the repository (and, also, so that the code doesn't throw noodles of E_STRICT errors when I run PHP 5.0). Should I simply give in and keep them as protected, or should I protect them and either 1. throw out their unit tests or 2. create a subclass that has a public interface for the protected method?
Thanks.
-
alex.barylski
- DevNet Evangelist
- Posts: 6267
- Joined: Tue Dec 21, 2004 5:00 pm
- Location: Winnipeg
- Maugrim_The_Reaper
- DevNet Master
- Posts: 2704
- Joined: Tue Nov 02, 2004 5:43 am
- Location: Ireland
It can depend on whether other code relied on PHP4 methods always being visible, or if method visibility was not considered as such during some part of development by someone. But yes, E_STRICT first, then do some refactoring to figure out which intended protected/private methods are being assumed as public by other parts of the system and make the necessary changes. I would only delete such unit tests gradually and check you have code coverage from the remaining tests.
- Ollie Saunders
- DevNet Master
- Posts: 3179
- Joined: Tue May 24, 2005 6:01 pm
- Location: UK
Sounds like you might have a bit of work on your hands there. I don't think anyone here is going to be able to suggest a way of getting round this problem that isn't going to involve looking at each problem on an individual basis and making some pretty big changes where necessary.
Bare in mind: if a method doesn't have side effects or affect it's object's state (or can be made that way easily) it can, with all likelihood, remain public even if it's not intended to be part of the public API without too much harm.
Bare in mind: if a method doesn't have side effects or affect it's object's state (or can be made that way easily) it can, with all likelihood, remain public even if it's not intended to be part of the public API without too much harm.
- Ambush Commander
- DevNet Master
- Posts: 3698
- Joined: Mon Oct 25, 2004 9:29 pm
- Location: New Jersey, US
(Hmm... it seems like topic reply notifications aren't working)
I've upgraded all the identifiers, using protected when it didn't break unit tests and public when it did (adding a @todo Make protected when I thought it wasn't very appropriate).
I've always thought making methods private/protected was a way of damage control: since no one else is allowed to use them, you can refactor with impunity (well, in the case of protected variables, you only have to worry about subclasses). It's not necessarily always practical to achieve 100% code coverage without testing some of the lower level functions, which makes me think they should be refactored to their own class. I'm also, however, trying to make sure my class count doesn't explode anymore than it is already (classes take up a lot of memory!)
Question: Is it a good practice to redefine protected methods as public in a "Testable" sub-class to unit test them?
I've upgraded all the identifiers, using protected when it didn't break unit tests and public when it did (adding a @todo Make protected when I thought it wasn't very appropriate).
I've always thought making methods private/protected was a way of damage control: since no one else is allowed to use them, you can refactor with impunity (well, in the case of protected variables, you only have to worry about subclasses). It's not necessarily always practical to achieve 100% code coverage without testing some of the lower level functions, which makes me think they should be refactored to their own class. I'm also, however, trying to make sure my class count doesn't explode anymore than it is already (classes take up a lot of memory!)
Question: Is it a good practice to redefine protected methods as public in a "Testable" sub-class to unit test them?
It is hard to believe for me those unit tests are good enough to rely on if should be public or protected..but who knows. It depend on scale of application.
Generaly I believe only interface should be tested...no private/protected should be part of tests.... only everything that is visible (public properties/methods)
Generaly I believe only interface should be tested...no private/protected should be part of tests.... only everything that is visible (public properties/methods)
- Ollie Saunders
- DevNet Master
- Posts: 3179
- Joined: Tue May 24, 2005 6:01 pm
- Location: UK
- Maugrim_The_Reaper
- DevNet Master
- Posts: 2704
- Joined: Tue Nov 02, 2004 5:43 am
- Location: Ireland
No it's not. Think of it this way - a class is a set of valued behaviours accessed from the public interface. Now, the important thing is that the behaviour is valuable, the implementation code is not (in fact it's generally extremely dynamic and changes as often as the Sun rises). Testing for the behaviour creates a valuable test (we certainly want to know if something valued breaks). Testing the implementation is literally duplication and pointless - if the behaviour is tested, and it relies on the implementation, then testing the implementation means we're doubling up (it's already executed by the tests of public behaviour if you check their code coverageQuestion: Is it a good practice to redefine protected methods as public in a "Testable" sub-class to unit test them?
I'd suspect testing protected/private methods is likely the number one problem with test suites in PHP today and you can spot it a mile off by the incredible number of tests even a small project ends up producing. Also subclassing just changes the SUT anyway - you're not really testing the original class anymore by cheating
The way I see them is that they're not important to the end user. All the user cares about is how to turn Lead into Gold (the internal process of the conversion is not worth examining unless it can be reused to turn Water into WineI've always thought making methods private/protected was a way of damage control: since no one else is allowed to use them, you can refactor with impunity (well, in the case of protected variables, you only have to worry about subclasses).
- John Cartwright
- Site Admin
- Posts: 11470
- Joined: Tue Dec 23, 2003 2:10 am
- Location: Toronto
- Contact: