I am actually reading theory about clean code and SOLID principles. I know understand well that we should program to an interface and not to an implementation.
So, I actually try to apply those principles to a little part of my code. I would like to have your advice or point of view so I can know if I am going in the good direction. I'll show you my previous code and my actual so you can visualize the evolution.
To start, i had a method in my controller to check some requirements for every step of an order process (4 steps that the user have to follow in the right order => 1 then 2 then 3 and then 4)
This is my old code :
private function isAuthorizedStep($stepNumber)
{
$isStepAccessAuthorized = TRUE;
switch($stepNumber) {
case self::ORDER_STEP_TWO: // ORDER_STEP_TWO = 2
if (!($_SESSION['actualOrderStep'] >= ORDER_STEP_ONE)) {
$isStepAccessAuthorized = FALSE;
}
break;
case self::ORDER_STEP_THREE:
if (!($_SESSION['actualOrderStep'] >= ORDER_STEP_TWO)) {
$isStepAccessAuthorized = FALSE;
}
break;
...
}
return $isStepAccessAuthorized;
}
public function orderStepTwo()
{
if ($this->isAuthorizedStep(self::ORDER_STEP_TWO) {
return;
}
... // do some stuff
// after all the verifications:
$_SESSION['actualOrderStep'] = ORDER_STEP_TWO
}
Trying to fit to SOLID principles, I splited my code following this logic:
- Extracting hard-coded logic from controllers to put it in classes (reusability)
- Using Dependency Injection and abstraction
interface RuleInterface {
public function matches($int);
}
class StepAccessControl
{
protected $rules;
public function __construct(array $rules)
{
foreach($rules as $key => $rule) {
$this->addRule($key, $rule);
}
}
public isAccessGranted($actualOrderStep)
{
$isAccessGranted = TRUE;
foreach($this->rules as $rule) {
if (!$rule->matches($actualOrderStep) {
$isAccessGranted = FALSE;
}
}
return $isAccessGranted;
}
public function addRule($key, RuleInterface $rule)
{
$this->rules[$key] = $rule;
}
}
class OrderStepTwoRule implements RuleInterface
{
public function matches($actualStep)
{
$matches = TRUE;
if (!($actualStep >= 1)) {
$isStepAccessAuthorized = FALSE;
}
return $matches;
}
}
class StepAccessControlFactory
{
public function build($stepNumber)
{
if ($stepNumber == 1) {
...
} elseif ($stepNumber == 2) {
$orderStepTwoRule = new OrderStepTwoRule();
return new StepAcessControl($orderStepTwoRule);
}...
}
}
and then in the controller :
public function stepTwoAction()
{
$stepAccessControlFactory = new StepAccessControlFactory();
$stepTwoAccessControl = $stepAccessControlFactory(2);
if (!$stepTwoAccessControl->isAccesGranted($_SESSION['actualOrderStep'])) {
return FALSE;
}
}
I would like to know if I get the spirit and if I am on the good way :)