patternphpMinor
A small extension to the reCaptcha library
Viewed 0 times
therecaptchaextensionsmalllibrary
Problem
I have been writing a small extension to the reCaptcha library.
What do you think? Is it anything to have? Anything I can improve/change?
class Captcha
{
private $config;
public $error;
public function __construct(array $config)
{
include './library/recaptchalib.php';
$this->config = $config;
}
public function render($error = null, $use_ssl = false)
{
return recaptcha_get_html($this->config['recaptcha']['public_key'],
$error, $use_ssl);
}
public function check()
{
$resp = recaptcha_check_answer ($this->config['recaptcha']['private_key'],
$_SERVER["REMOTE_ADDR"],
$_POST["recaptcha_challenge_field"],
$_POST["recaptcha_response_field"]);
if (!$resp->is_valid) {
$this->error = $resp->error;
return false;
}
}
}What do you think? Is it anything to have? Anything I can improve/change?
Solution
-
The
-
What's the point of returning with
-
The
References:
The
$error field should be private (Why use getters and setters?). Currently users of the Captcha class can modify the value of the $error field directly which. It's not a good idea to allow them to do this.-
What's the point of returning with
false in the check function when the response is not valid while if it's valid the function does not return with true?-
The
render and check functions seems somehow temporarily coupled. See: Clean Code (by Robert C. Martin), G31: Hidden Temporal CouplingsReferences:
- Effective Java Second Edition, Item 38: Check parameters for validity (I know that this is a Java book but this chapter applicable to PHP too.)
- Function/method argument validation standards
Context
StackExchange Code Review Q#12636, answer score: 3
Revisions (0)
No revisions yet.