php function
is it correct to write a function like this?
function clean($data) {
$data = htmlspecialchars($data);
$data = $mysqli->real_escape_string($data);
return $data;
}ASKER CERTIFIED SOLUTION
membership
This solution is only available to members.
To access this solution, you must be a member of Experts Exchange.
Two things worth noting here.
1. Functions encapsulate "scope." The "scope" is the set of variables that are visible inside the function. Since $mysqli is not defined in the function code, it is not in the scope of the function. As Julian points out, it must therefore be injected into the function. We call this "dependency injection" because code inside the function depends on the $mysqli variable.
http://php.net/manual/en/language.variables.scope.php
https://www.experts-exchange.com/articles/18210/Software-Design-Dependencies.html
https://www.experts-exchange.com/articles/19999/PHP-Design-Avoiding-Globals-with-Dependency-Injection.html
2. Functions (like classes) should do one thing only. In the instant case, this function mungs the $data variable twice, and in discordant ways. The first mung changes special characters into entities. The second mung changes the string, injecting escape sequences that remove the programmatic meaning of things like quotes and apostrophes.
The second mung, escape(), is used before sending data to the database engine.
The first mung, htmlspecialchars() is used before sending data to the client browser.
This leads me to ask, "which is it?" If you're preparing data for use in a query string, your script is part of the data model. But if you're preparing data for use in a browser display, your script is part of the view. The MVC design pattern dictates isolation of the model and the view (separation of concerns), yet this function is doing two things at once, and is therefore not appropriate for either the model or the view!
If you want to get a better idea of what a clean() function should be doing, just tell us in plain language where the data is coming from and where you want to use the data. There are fairly well-understood filter and sanitize patterns that we can discuss.
1. Functions encapsulate "scope." The "scope" is the set of variables that are visible inside the function. Since $mysqli is not defined in the function code, it is not in the scope of the function. As Julian points out, it must therefore be injected into the function. We call this "dependency injection" because code inside the function depends on the $mysqli variable.
http://php.net/manual/en/language.variables.scope.php
https://www.experts-exchange.com/articles/18210/Software-Design-Dependencies.html
https://www.experts-exchange.com/articles/19999/PHP-Design-Avoiding-Globals-with-Dependency-Injection.html
2. Functions (like classes) should do one thing only. In the instant case, this function mungs the $data variable twice, and in discordant ways. The first mung changes special characters into entities. The second mung changes the string, injecting escape sequences that remove the programmatic meaning of things like quotes and apostrophes.
The second mung, escape(), is used before sending data to the database engine.
The first mung, htmlspecialchars() is used before sending data to the client browser.
This leads me to ask, "which is it?" If you're preparing data for use in a query string, your script is part of the data model. But if you're preparing data for use in a browser display, your script is part of the view. The MVC design pattern dictates isolation of the model and the view (separation of concerns), yet this function is doing two things at once, and is therefore not appropriate for either the model or the view!
If you want to get a better idea of what a clean() function should be doing, just tell us in plain language where the data is coming from and where you want to use the data. There are fairly well-understood filter and sanitize patterns that we can discuss.
ASKER