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\$
  • \$\begingroup\$ Could you add more context to your code, the more we have the better review we can make! \$\endgroup\$ – IEatBagels Oct 22 '15 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 Nov 6 '15 at 9:59
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
    \$\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 Nov 6 '15 at 10:04

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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