views:

98

answers:

5

Hello,

normally, all sane developers are trying to secure input of all public methods (casting to proper types, validating, sanitizing etc.)

My question is: are you in your code validating also parameters passed to protected / private methods? In my opinion it is not necessary, if you securize properly parameters of public methods and return values from outside (other classes, db, user input etc...).

But I am constantly facing frameworks and apps (ie. prestashop to name one) where validation is often repeated in method call, in method body and once again for securize returned value - which, I think, is creating performace overhead and is also a sign of bad design.

A: 

Exactly - if you design your app well then it should not be necessary.

Jakub Hampl
+3  A: 

For protected, I think you should validate them since the method could be overridden or called from another class later and you can't assume valid inputs to the method. This is especially true if this is a component that is going to be used by other applications.

For private, I think it's a waste because you are in control of what is being passed to the methods, so that data should be validated before you ever call the private method.

dcp
+2  A: 

If you adhere to the opinion that public APIs should have implementations that defend themselves against bad parameters, you criterion should not be the visibility of the methods, but whether the user of the API is going to directly call that method (or indirectly call it through another one which defers the validation).

Examples of methods that ought to do validation:

class A {
    protected final function myMethodDefaultImplementation(...) {
        /* subclasses can just call this method in their myMethod implementations */
        /* should do validation */
        ...
    }
    protected abstract myMethod(...);

    public function orderByDate() {
        return $this->orderBy(ORDER_BY_DATE)
    }

    private function orderBy($crit) {
        /* should do validation */
        ...
    }
}
Artefacto
A: 

Only sanitize input at the last possible chance. I don't see how OO semantics make this any different.

For instance if for some reason you can't use parameterized queries or an ORM (ony example I can think of at the moment :), you'd write the function like this:

function getname($id) {
    $id = intval($id);
    mysql_query("SELECT * FROM users WHERE id = $id");
    ...
}

Now it's impossible for any code to call this function and cause unexpected results.

Longpoke
Why downvote? This is how it's done. It makes no sense to create API that let you inject SQL. Not to mention this is **exactly** the same as the accepted solution.
Longpoke
+1  A: 

I would say it does not matter what type of a method it is (public, private, protected), you take proper precautions whenever that is needed without looking at the visibility keyword.

Kai Sellgren