Uh oh!
There was an error while loading. Please reload this page.
Enhancement: Extract phpweb\Spam\Challenge - #664
Conversation
a076df1 to
dedb915CompareUh oh!
There was an error while loading. Please reload this page.
kamil-tekiela
left a comment
There was a problem hiding this comment.
I love this. It helps with the readability a lot.
I am not a fan of static methods though. They make sense for create and isValidAnswer but everything else doesn't really need to be a public static method. I believe that we should test all public method with a dedicated test if possible. In this case, we don't access these methods from anywhere else outside the scope of the class.
I would also encourage you to specify all parameter and return types. This should also allow you to change $strict to true.
| $a = rand(0, 9); | ||
| $argumentOne = self::NUM[$a]; |
There was a problem hiding this comment.
One-letter variable names aren't very good. Would you be able to come up with more descriptive name?
There was a problem hiding this comment.
I think it's okay here (besides the variables are local, they're only used in the next statement). Although, we could inline the rand() call, and drop $a altogether (probably not a good for $b below).
| public static function plus($a, $b) { | ||
| return $a + $b; | ||
| } | ||
| public static function gen_plus($a) { | ||
| return rand(0, 9 - $a); | ||
| } | ||
| public static function minus($a, $b) { | ||
| return $a - $b; | ||
| } | ||
| public static function gen_minus($a) { | ||
| return rand(0, $a); | ||
| } | ||
| public static function print_infix($name, $a, $b) { | ||
| return "$a $name $b"; | ||
| } | ||
| public static function print_prefix($name, $a, $b) { | ||
| return "$name($a, $b)"; | ||
| } |
There was a problem hiding this comment.
These do not need to be public or even static (except minus and plus). Could you make them all instance methods and then call them from the getters or from the constructor?
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
22bb34e to
05aef42Compare05aef42 to
a88faf2Compare🚀 Deployed on https://web-php-pr-664.preview.thephp.foundation |
This pull request
phpweb\Spam\Challenge