3

It is OK to have methods depend one on each other inside a class, methods that are encapsulated together? Does this affect unit testing? or the term Unit is addressing to the whole class and not to the class own methods.

It is OK to do something like this:

<?php

class foo {
    protected $baz;
    protected $bar;

    private function checkBaz($baz) {
        // do some checking
    }
    private function checkBar($bar) {
        // do some checking
    }
    public function setBaz($baz) {
        if($this->checkBaz($baz)){
            $this->baz = $baz;
        }
    }
    public function setBar($bar) {
        if($this->checkBar($bar)){
            $this->bar = $bar;
        }
    }
}

I'm thinking if I want to use a bound method from this class to another class I have to rewrite the method a little bit, I was wandering if it is overhead to make methods somehow encapsulated like inserting functionality in to method parameters, Is this a common practice in OOP or should I stick to the thinking that a class is an encapsulation of high cohesion method and properties and see it like a whole reusable and testing Unit.

Starlays
  • 999
  • 1
  • 10
  • 27
  • 1
    What you want to test is the public (or interface) of your class. (btw the _ in method names is not according to the PSR standard). As long as you can mock your dependencies, you won't have a problem. You test if your public method works and do not care how they work. – Anyone Feb 11 '14 at 09:06
  • @Anyone PSR isn't a standard. It's not sanctioned by the PHP group. They're just a group of guys with opinions. – GordonM Feb 11 '14 at 09:08
  • @Anyone removed the `_` in front of method name. Not all my code is written according do PSR standard, it something that I have made on the moment. – Starlays Feb 11 '14 at 09:09
  • @GordonM I'm not saying it's a must. However, It's the only standard there is which is properly defined and followed by Composer and Symfony2. thereore I recommend following it, even if you don't like the codestyle (I don't agree with some things, but standard is standard). Life would be so much easier if everyone would just follow it :P – Anyone Feb 11 '14 at 09:43
  • @Anyone That's my point, it's not a standard. At best it's a recommendation. – GordonM Feb 11 '14 at 15:36
  • @GordonM What makes a standard a standard then? Anyone telling the world how they do things, and some people agree and do it the same way. That's about as standard as it can get on the internet (which is built upon this same mechanism). – Sven Feb 11 '14 at 22:18

3 Answers3

2

Yes, calling methods internally in a class is absolutely fine. In fact, it's encouraged. The mere presence of protected and private methods should be a dead giveaway. You can only call those from within the class, from somewhere that needs to be triggered by a public method, so yes, your example if perfectly in line with how those methods are supposed to be used.

The only thing that matters in general and in terms of unit testing is how the object as a whole behaves. Its public components are important, that's what other code interacts with and it's what other code observes. Whatever kinds of internal calls your object/class makes is perfectly up to its own discretion. If it helps your class structure to put certain code in separate methods and hide them from the outside world, so be it.

deceze
  • 471,072
  • 76
  • 664
  • 811
2

Yes, performing method calls like this is perfectly fine. They're marked as private so they remain encapsulated within your class and unexposed to the outside world.

If these method calls started to cause the class/object to deviate its main purpose, breaking single responsibility principle, then it would be worth looking at breaking the class into finer units and injecting this new dependency via dependency injection. An example of this would be if the checkBaz method was performing a database look-up.

In terms of unit testing, what you really want to do is program to an interface not implementation by creating an interface for your foo class and implementing it. This means whenever you program to it you're programming to a contract - allowing you to easily mock your IFoo implementation.

eg:

class Foo implements IFoo {

}

Your interface will look like this:

interface IFoo
{
    public function setBaz($baz)
    public function setBar($bar)
}

I would also recommend following the PEAR coding standard and make your class names uppercase first. Though if this is a decision made by you or your team then that's entirely your call.

Community
  • 1
  • 1
Joseph Woodward
  • 8,837
  • 5
  • 39
  • 61
1

There's nothing wrong with non-public methods. In fact, a class should only export the bare minimum of public methods to allow other classes to make use of its services. The more external classes know about the inner workings of your class, the more difficult it is to make changes to the inner workings of your class without upsetting external dependencies.

The non-public methods should be tested by being called indirectly. There should be a circumstance under which every method gets called eventually, even if it's through indirect calling (public method calls private method, etc). If a method is never executed in a unit test then that's a good indicator that it is in fact dead code and can be removed from the class altogether.

You can in theory directly test the protected methods of a class by subclassing it and elevating the protected methods to public, but that's not recommended because it exposes internal workings of your class and you don't really want to do that.

GordonM
  • 29,211
  • 15
  • 77
  • 126