Giving meaninful name to condition

I have a Request and in this request there's an if that verifies if the request is a create or an update. In the first case, the rules change for making the password required for a user, and, in the second, they're nullable. I have put my condition in a separate function, to be able to give this condition a more meaninful name (as my code bellow shows). I want to come back to this place in the future and be able to understand what it does at first glance. My question is, then, is this correct or too much? I feel like this is necessary, but also feel a bit bad for creating another function for this.

....
public function rules()
  {
    if ($this->__isCreateAction()) {
      $rules = [
        'email' => 'required|string|email|unique:users,email',
        'name' => 'required|string|max:255',
        'password' => 'required|string|min:6|max:12',
        'password_confirmation' => 'required|string|min:6|max:12'
      ];
    } else {
      $rules = [
        'email' => 'required|string|email|unique:users,email',
        'name' => 'required|string|max:255',
        'password' => 'required_with:password_confirmation|string|min:6|max:12',
        'password_confirmation' => 'required_with:password|string|min:6|max:12'
      ];
    }

    return $rules;
  }

  private function __isCreateAction()
  {
    return $this->isMethod('POST');
  }
....

jlrdw

3 years ago

Level 75

I wouldn't have password on a regular edit form, rather do a "Change password" separate altogether. But just a suggestion.

And just comment above the code as to what it does, look at the vendor folder and how Taylor comments code.

1 like

tykus

3 years ago

Level 104

Personally, I would try to avoid duplication, e.g. something like this:

public function rules()
{
  return [
    'email' => ['required', 'string', 'email', 'unique:users'],
    'name' => ['required', 'string', 'max:255'],
    'password' => [
      $this->isMethod('post') ? 'required' : 'nullable',
      'string',
      'min:6',
      'max:12',
	  'confirmed',
    ],
  ];
}

Note that there is a confirmed validation rule to handle the password_confirmation field so those rules are completely unnecessary. Now you only have one different rule; required whenever creating or nullable whenever updating; does this really need a separate method?

Note also, there a specific Password validation rules for more robust validation!

1 like

Please or to participate in this conversation.