Coding conventions for ConfirmedPasswordField

forms/ConfirmedPasswordField.php

@@ -1,8 +1,10 @@
 <?php
+
 /**
  * Two masked input fields, checks for matching passwords.
- * Optionally hides the fields by default and shows
+ *
- * a link to toggle their visibility.
+ * Optionally hides the fields by default and shows a link to toggle their 
+ * visibility.
  * 
  * @package forms
  * @subpackage fields-formattedinput
@@ -39,11 +41,12 @@ class ConfirmedPasswordField extends FormField {
 	public $canBeEmpty = false;
 	
 	/**
-	 * If set to TRUE, the "password" and "confirm password"
+	 * If set to TRUE, the "password" and "confirm password" form fields will 
-	 * formfields will be hidden via CSS and JavaScript by default,
+	 * be hidden via CSS and JavaScript by default, and triggered by a link. 
-	 * and triggered by a link. An additional hidden field
+	 *
-	 * determines if showing the fields has been triggered,
+	 * An additional hidden field determines if showing the fields has been 
-	 * and just validates/saves the input in this case.
+	 * triggered and just validates/saves the input in this case.
+	 *
 	 * This behaviour works unobtrusively, without JavaScript enabled
 	 * the fields show, validate and save by default.
 	 * 
@@ -52,8 +55,7 @@ class ConfirmedPasswordField extends FormField {
 	protected $showOnClick = false;
 	
 	/**
-	 * Title for the link that triggers
+	 * Title for the link that triggers the visibility of password fields.
-	 * the visibility of password fields.
 	 *
 	 * @var string
 	 */
@@ -102,6 +104,11 @@ class ConfirmedPasswordField extends FormField {
 		$this->setValue($value);
 	}
 	
+	/**
+	 * @param array $properties
+	 *
+	 * @return string
+	 */
 	public function Field($properties = array()) {
 		Requirements::javascript(FRAMEWORK_DIR . '/thirdparty/jquery/jquery.js');
 		Requirements::javascript(FRAMEWORK_DIR . '/javascript/ConfirmedPasswordField.js');
@@ -146,65 +153,91 @@ class ConfirmedPasswordField extends FormField {
 	}
 	
 	/**
-	 * Can be empty is a flag that turns on/off empty field checking.
+	 * Can be empty is a flag that turns on / off empty field checking.
+	 *
 	 * For example, set this to false (the default) when creating a user account,
-	 * and true 
+	 * and true when displaying on an edit form.
+	 *
+	 * @param boolean $value
+	 * 
+	 * @return ConfirmedPasswordField
 	 */
 	public function setCanBeEmpty($value) {
 		$this->canBeEmpty = (bool)$value;
+
 		return $this;
 	}
 	
 	/**
-	 * The title on the link which triggers display of the
+	 * The title on the link which triggers display of the "password" and 
-	 * "password" and "confirm password" formfields.
+	 * "confirm password" formfields. Only used if {@link setShowOnClick()} 
-	 * Only used if {@link setShowOnClick()} is set to TRUE.
+	 * is set to TRUE.
 	 * 
-	 * @param $title
+	 * @param string $title
+	 *
+	 * @return ConfirmedPasswordField
 	 */
 	public function setShowOnClickTitle($title) {
 		$this->showOnClickTitle = $title;
+
 		return $this;
 	}
 	
 	/**
-	 * @return string
+	 * @return string $title
 	 */
 	public function getShowOnClickTitle() {
 		return $this->showOnClickTitle;
 	}
 	
+	/**
+	 * @param string $title
+	 *
+	 * @return ConfirmedPasswordField
+	 */
 	public function setRightTitle($title) {
 		foreach($this->children as $field) {
 			$field->setRightTitle($title);
 		}
+
 		return $this;
 	}
 	
 	/**
-	 * @param array: 2 entrie array with the customised title for each of the 2 children.
+	 * @param array $titles 2 entry array with the customized title for each 
+	 *						of the 2 children.
+	 *
+	 * @return ConfirmedPasswordField
 	 */
 	public function setChildrenTitles($titles) {
-		if(is_array($titles)&&count($titles)==2){
+		if(is_array($titles) && count($titles) == 2) {
 			foreach($this->children as $field) {
-				if(isset($titles[0])){
+				if(isset($titles[0])) {
 					$field->setTitle($titles[0]);
+
 					array_shift($titles);		
 				}
 			}
 		}
+
 		return $this;
 	}
 	
 	/**
-	 * Value is sometimes an array, and sometimes a single value, so we need to handle both cases
+	 * Value is sometimes an array, and sometimes a single value, so we need 
+	 * to handle both cases.
+	 *
+	 * @param mixed $value
+	 *
+	 * @return ConfirmedPasswordField
 	 */
 	public function setValue($value) {
 		if(is_array($value)) {
 			if($value['_Password'] || (!$value['_Password'] && !$this->canBeEmpty)) {
 				$this->value = $value['_Password'];
 			}
-			if($this->showOnClick && isset($value['_PasswordFieldVisible'])){
+
+			if($this->showOnClick && isset($value['_PasswordFieldVisible'])) {
 				$this->children->fieldByName($this->getName() . '[_PasswordFieldVisible]')
 					->setValue($value['_PasswordFieldVisible']);
 			}
@@ -213,29 +246,40 @@ class ConfirmedPasswordField extends FormField {
 				$this->value = $value;
 			}
 		}
-		$this->children->fieldByName($this->getName() . '[_Password]')->setValue($this->value);
+
-		$this->children->fieldByName($this->getName() . '[_ConfirmPassword]')->setValue($this->value);
+		$this->children->fieldByName($this->getName() . '[_Password]')
+			->setValue($this->value);
+
+		$this->children->fieldByName($this->getName() . '[_ConfirmPassword]')
+			->setValue($this->value);
 
 		return $this;
 	}
 
 	/**
-	 * Determines if the field was actually
+	 * Determines if the field was actually shown on the client side - if not, 
-	 * shown on the clientside - if not,
 	 * we don't validate or save it.
 	 * 
-	 * @return bool
+	 * @return boolean
 	 */
 	public function isSaveable() {
 		$isVisible = $this->children->fieldByName($this->getName() . '[_PasswordFieldVisible]');
+
 		return (!$this->showOnClick || ($this->showOnClick && $isVisible && $isVisible->Value()));
 	}
 	
+	/**
+	 * @param Validator $validator
+	 *
+	 * @return boolean
+	 */
 	public function validate($validator) {
 		$name = $this->name;
 		
 		// if field isn't visible, don't validate
-		if(!$this->isSaveable()) return true; 
+		if(!$this->isSaveable()) {
+			return true;
+		}
 		
 		$passwordField = $this->children->fieldByName($name.'[_Password]');
 		$passwordConfirmField = $this->children->fieldByName($name.'[_ConfirmPassword]');
@@ -246,16 +290,26 @@ class ConfirmedPasswordField extends FormField {
 		
 		// both password-fields should be the same
 		if($value != $passwordConfirmField->Value()) {
-			$validator->validationError($name, _t('Form.VALIDATIONPASSWORDSDONTMATCH',"Passwords don't match"),
+			$validator->validationError(
-				"validation", false);
+				$name, 
+				_t('Form.VALIDATIONPASSWORDSDONTMATCH',"Passwords don't match"),
+				"validation", 
+				false
+			);
+
 			return false;
 		}
 
 		if(!$this->canBeEmpty) {
 			// both password-fields shouldn't be empty
 			if(!$value || !$passwordConfirmField->Value()) {
-				$validator->validationError($name, _t('Form.VALIDATIONPASSWORDSNOTEMPTY', "Passwords can't be empty"),
+				$validator->validationError(
-					"validation", false);
+					$name, 
+					_t('Form.VALIDATIONPASSWORDSNOTEMPTY', "Passwords can't be empty"),
+					"validation", 
+					false
+				);
+
 				return false;
 			}
 		}
@@ -302,21 +356,25 @@ class ConfirmedPasswordField extends FormField {
 					"validation", 
 					false
 				);
+
 				return false;
 			}
 		}
+
 		return true;
 	}
 	
 	/**
-	 * Only save if field was shown on the client,
+	 * Only save if field was shown on the client, and is not empty.
-	 * and is not empty.
 	 *
-	 * @param DataObject $record
+	 * @param DataObjectInterface $record
-	 * @return bool
+	 *
+	 * @return boolean
 	 */
 	public function saveInto(DataObjectInterface $record) {
-		if(!$this->isSaveable()) return false;
+		if(!$this->isSaveable()) {
+			return false;
+		}
 		
 		if(!($this->canBeEmpty && !$this->value)) {
 			parent::saveInto($record);
@@ -324,7 +382,9 @@ class ConfirmedPasswordField extends FormField {
 	}
 	
 	/**
-	 * Makes a pretty readonly field with some stars in it
+	 * Makes a read only field with some stars in it to replace the password
+	 *
+	 * @return ReadonlyField
 	 */
 	public function performReadonlyTransformation() {
 		$field = $this->castedCopy('ReadonlyField')