0
\$\begingroup\$

I'm trying to find a way to make my code easeier to read, so i found that major part of the code is validation trough if, ifelse:

    if (!$this -> is_valid_username($username)) {
        $this -> Session -> setFlash('Enter valid client username, Error 1.', 'default', array(), 'errorlogaddclient');
        $this -> redirect('/dashboard/settings');
        exit ;
    }
    if (!$this -> is_valid_password($password) || !$this -> is_valid_defpass($password)) {
        $this -> Session -> setFlash('Enter valid password, Error 2.', 'default', array(), 'errorlogaddclient');
        $this -> redirect('/dashboard/settings');
        exit ;
    }
    if ($this -> is_already_client($user)) {
        $this -> Session -> setFlash("User is already someone's client, Error 3", 'default', array(), 'errorlogaddclient');
        $this -> redirect('/dashboard/settings');
        exit ;
    }//ect

so what is proper/better way to do this

\$\endgroup\$
2
  • \$\begingroup\$ Could you add more context to your code, the more we have the better review we can make! \$\endgroup\$
    – IEatBagels
    Commented Oct 22, 2015 at 13:03
  • 1
    \$\begingroup\$ The way you've implemented validation here is completely different to how the framework does it. your code should be, in principle if (!$this->User->validates()) { ... \$\endgroup\$
    – AD7six
    Commented Nov 6, 2015 at 9:59

1 Answer 1

2
\$\begingroup\$

I would split up the code into objects, with good object oriented patterns. This should result in objects that have very distinct responsibilities.

The pattern I think you should primarily look into is the "Proxy" pattern (See "Design Patterns" book by "Gang of four".) It can be used to extract the code that ensures security, for example. It makes the security code independent from the domain logic / data access / user interface.

It may mean refactoring a lot of code, but once done, you can then write thorough unit-tests for the security proxy in isolation, giving you a rock solid foundation.

Write tests, then write the testable code, (i.e. isolated) then refactor to the point where it's neat and tidy.

\$\endgroup\$
1
  • 1
    \$\begingroup\$ This isn't good advice in the context of the question. The documentation for CakePHP indicates how to do validation - and it looks nothing like the question. Step one should always be to use code as designed. \$\endgroup\$
    – AD7six
    Commented Nov 6, 2015 at 10:04

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.