Permalink
Browse files

Removing the need for first param in CakeRule constructor

  • Loading branch information...
lorenzo committed May 5, 2012
1 parent 3199b90 commit 877e6c0f66279027e53f173e2291f2697d3f2198
@@ -37,13 +37,6 @@ class CakeRule {
*/
protected $_valid = true;
-/**
- * Holds the index under which the Validator was attached
- *
- * @var mixed
- */
- protected $_index = null;
-
/**
* Create or Update transaction?
*
@@ -118,10 +111,8 @@ class CakeRule {
* Constructor
*
* @param array $validator [optional] The validator properties
- * @param mixed $index [optional]
*/
- public function __construct($index = null, $validator = array()) {
- $this->_index = $index;
+ public function __construct($validator = array()) {
$this->_addValidatorProps($validator);
}
@@ -286,10 +277,6 @@ public function getOptions($key) {
return $this->_passedOptions[$key];
}
- public function getName() {
- return $this->_index;
- }
-
/**
* Sets the rule properties from the rule entry in validate
*
@@ -85,7 +85,7 @@ public function __construct($fieldName, $ruleSet) {
}
foreach ($ruleSet as $index => $validateProp) {
- $this->_rules[$index] = new CakeRule($index, $validateProp);
+ $this->_rules[$index] = new CakeRule($validateProp);
}
$this->ruleSet = $ruleSet;
}
@@ -116,7 +116,7 @@ public function setValidationDomain($validationDomain) {
*/
public function validate($data, $isUpdate = false) {
$errors = array();
- foreach ($this->getRules() as $rule) {
+ foreach ($this->getRules() as $name => $rule) {
$rule->isUpdate($isUpdate);
if ($rule->skip()) {
continue;
@@ -131,7 +131,7 @@ public function validate($data, $isUpdate = false) {
}
if ($checkRequired || !$rule->isValid()) {
- $errors[] = $this->_processValidationResponse($rule);
+ $errors[] = $this->_processValidationResponse($name, $rule);
if ($rule->isLast()) {
break;
}
@@ -193,11 +193,12 @@ public function setRules($rules = array(), $mergeVars = true) {
/**
* Fetches the correct error message for a failed validation
*
+ * @param string $name the name of the rule as it was configured
+ * @param CakeRule $rule the object containing validation information
* @return string
*/
- protected function _processValidationResponse($rule) {
+ protected function _processValidationResponse($name, $rule) {
$message = $rule->getValidationResult();
- $name = $rule->getName();
if (is_string($message)) {
return $message;
}
@@ -74,7 +74,7 @@ public function testIsValid() {
);
$methods = array();
- $Rule = new CakeRule('notEmpty', $def);
+ $Rule = new CakeRule($def);
$Rule->process('fieldName', $data, $methods);
$this->assertFalse($Rule->isValid());
@@ -94,7 +94,7 @@ public function testCustomMethods() {
);
$methods = array('mytestrule' => array($this, 'myTestRule'));
- $Rule = new CakeRule('custom', $def);
+ $Rule = new CakeRule($def);
$Rule->process('fieldName', $data, $methods);
$this->assertFalse($Rule->isValid());
@@ -114,19 +114,19 @@ public function testCustomMethods() {
*/
public function testIsRequired() {
$def = array('rule' => 'notEmpty', 'required' => true);
- $Rule = new CakeRule('required', $def);
+ $Rule = new CakeRule($def);
$this->assertTrue($Rule->isRequired());
$def = array('rule' => 'notEmpty', 'required' => false);
- $Rule = new CakeRule('required', $def);
+ $Rule = new CakeRule($def);
$this->assertFalse($Rule->isRequired());
$def = array('rule' => 'notEmpty', 'required' => 'create');
- $Rule = new CakeRule('required', $def);
+ $Rule = new CakeRule($def);
$this->assertTrue($Rule->isRequired());
$def = array('rule' => 'notEmpty', 'required' => 'update');
- $Rule = new CakeRule('required', $def);
+ $Rule = new CakeRule($def);
$this->assertFalse($Rule->isRequired());
$Rule->isUpdate(true);
@@ -108,18 +108,18 @@ public function testGetRules() {
public function testSetRule() {
$rules = array('notEmpty' => array('rule' => 'notEmpty', 'message' => 'Can not be empty'));
$Field = new CakeValidationSet('title', $rules);
- $Rule = new CakeRule('notEmpty', $rules['notEmpty']);
+ $Rule = new CakeRule($rules['notEmpty']);
$this->assertEquals($Rule, $Field->getRule('notEmpty'));
$rules = array('validEmail' => array('rule' => 'email', 'message' => 'Invalid email'));
- $Rule = new CakeRule('validEmail', $rules['validEmail']);
+ $Rule = new CakeRule($rules['validEmail']);
$Field->setRule('validEmail', $Rule);
$result = $Field->getRules();
$this->assertEquals(array('notEmpty', 'validEmail'), array_keys($result));
$rules = array('validEmail' => array('rule' => 'email', 'message' => 'Other message'));
- $Rule = new CakeRule('validEmail', $rules['validEmail']);
+ $Rule = new CakeRule($rules['validEmail']);
$Field->setRule('validEmail', $Rule);
$result = $Field->getRules();
$this->assertEquals(array('notEmpty', 'validEmail'), array_keys($result));
@@ -141,10 +141,10 @@ public function testSetRule() {
public function testSetRules() {
$rule = array('notEmpty' => array('rule' => 'notEmpty', 'message' => 'Can not be empty'));
$Field = new CakeValidationSet('title', $rule);
- $RuleEmpty = new CakeRule('title', $rule['notEmpty'], 'notEmpty');
+ $RuleEmpty = new CakeRule($rule['notEmpty']);
$rule = array('validEmail' => array('rule' => 'email', 'message' => 'Invalid email'));
- $RuleEmail = new CakeRule('email', $rule['validEmail'], 'validEmail');
+ $RuleEmail = new CakeRule($rule['validEmail']);
$rules = array('validEmail' => $RuleEmail);
$Field->setRules($rules, false);

3 comments on commit 877e6c0

Member

ADmad replied May 7, 2012

I would suggest renaming class CakeRule to CakeValidationRule. CakeRule is too generic and we could potentially need rules at other places within the framework.

Member

ADmad replied May 7, 2012

Also a class name by itself should give you at least some idea of what it does or what it relates too. For someone new to the framework the name CakeRule by itself would give no clue about what it is or does.

Member

ADmad replied May 7, 2012

I also wonder if prefixing these class names with "Cake" is really necessary. I don't think ValidationRule, ValidationSet have a great possibility of name collision.

Please sign in to comment.