cleanup of generics and form valuecomparison

CRM/CivirulesConditions/Form/ValueComparison.php

 <?php
+/**
+ * Class for CiviRules ValueComparison Form
+ *
+ * @author Jaap Jansma (CiviCooP) <jaap.jansma@civicoop.org>
+ * @license AGPL-3.0
+ */
 
 class CRM_CivirulesConditions_Form_ValueComparison extends CRM_Core_Form {
 
   protected $ruleConditionId = false;
 
-  function preProcess() {
+  /**
+   * Overridden parent method to perform processing before form is build
+   *
+   * @access public
+   */
+  public function preProcess() {
     $this->ruleConditionId = CRM_Utils_Request::retrieve('rule_condition_id', 'Integer');
     parent::preProcess();
   }
 
-  function buildQuickForm() {
+  /**
+   * Overridden parent method to build form
+   *
+   * @access public
+   */
+  public function buildQuickForm() {
     $this->setFormTitle();
 
     $this->add('hidden', 'rule_condition_id');
@@ -29,6 +45,12 @@ class CRM_CivirulesConditions_Form_ValueComparison extends CRM_Core_Form {
       array('type' => 'cancel', 'name' => ts('Cancel'))));
   }
 
+  /**
+   * Overridden parent method to set default values
+   *
+   * @return array $defaultValues
+   * @access public
+   */
   public function setDefaultValues() {
     $data = array();
     $defaultValues = array();
@@ -47,13 +69,19 @@ class CRM_CivirulesConditions_Form_ValueComparison extends CRM_Core_Form {
     return $defaultValues;
   }
 
+  /**
+   * Overridden parent method to process form data after submission
+   *
+   * @throws Exception when rule condition not found
+   * @access public
+   */
   public function postProcess() {
-    $rule_id = 0;
+    $ruleId = 0;
     $ruleCondition = new CRM_Civirules_BAO_RuleCondition();
     $ruleCondition->id = $this->ruleConditionId;
     $condition_label = '';
     if ($ruleCondition->find(true)) {
-      $rule_id = $ruleCondition->rule_id;
+      $ruleId = $ruleCondition->rule_id;
       $condition = new CRM_Civirules_BAO_Condition();
       $condition->id = $ruleCondition->condition_id;
       if ($condition->find(true)) {
@@ -69,27 +97,33 @@ class CRM_CivirulesConditions_Form_ValueComparison extends CRM_Core_Form {
     $ruleCondition->save();
 
     $session = CRM_Core_Session::singleton();
-    $session->setStatus('Condition '.$condition_label.' parameters updated to CiviRule '.CRM_Civirules_BAO_Rule::getRuleLabelWithId($rule_id),
+    $session->setStatus('Condition '.$condition_label.' parameters updated to CiviRule '
+      .CRM_Civirules_BAO_Rule::getRuleLabelWithId($ruleId),
       'Condition parameters updated', 'success');
 
-    $redirectUrl = CRM_Utils_System::url('civicrm/civirule/form/rule', 'action=update&id='.$rule_id, TRUE);
+    $redirectUrl = CRM_Utils_System::url('civicrm/civirule/form/rule', 'action=update&id='.$ruleId, TRUE);
     CRM_Utils_System::redirect($redirectUrl);  }
 
+  /**
+   * Method to set the form title
+   *
+   * @access protected
+   */
   protected function setFormTitle() {
-    $condition_label = '';
+    $conditionLabel = '';
     $ruleCondition = new CRM_Civirules_BAO_RuleCondition();
     $ruleCondition->id = $this->ruleConditionId;
     if ($ruleCondition->find(true)) {
       $condition = new CRM_Civirules_BAO_Condition();
       $condition->id = $ruleCondition->condition_id;
       if ($condition->find(true)) {
-        $condition_label = $condition->label;
+        $conditionLabel = $condition->label;
       }
     }
 
     $title = 'CiviRules Edit Condition parameters';
-    $this->assign('ruleConditionHeader', 'Edit Condition '.$condition_label.' of CiviRule '.CRM_Civirules_BAO_Rule::getRuleLabelWithId($ruleCondition->rule_id));
+    $this->assign('ruleConditionHeader', 'Edit Condition '.$conditionLabel.' of CiviRule '
+      .CRM_Civirules_BAO_Rule::getRuleLabelWithId($ruleCondition->rule_id));
     CRM_Utils_System::setTitle($title);
   }
-
 }
\ No newline at end of file

CRM/CivirulesConditions/Generic/FieldChanged.php

 <?php
+/**
+ * Abstract Class for CiviRules Generic Field Changed condition
+ *
+ * @author Jaap Jansma (CiviCooP) <jaap.jansma@civicoop.org>
+ * @license AGPL-3.0
+ */
 
 abstract class CRM_CivirulesConditions_Generic_FieldChanged extends CRM_Civirules_Condition {
 
@@ -6,15 +12,27 @@ abstract class CRM_CivirulesConditions_Generic_FieldChanged extends CRM_Civirule
    * Returns name of entity
    *
    * @return string
+   * @access protected
+   * @abstract
    */
   abstract protected function getEntity();
 
   /**
    * Returns name of the field
+   *
    * @return string
+   * @access protected
+   * @abstract
    */
   abstract protected function getField();
 
+  /**
+   * Method to check if the condition is valid
+   *
+   * @param object CRM_Civirules_EventData_EventData $eventData
+   * @return bool
+   * @access public
+   */
   public function isConditionValid(CRM_Civirules_EventData_EventData $eventData) {
     //not the right event. The event data should contain also
     if (!$eventData instanceof CRM_Civirules_EventData_Interface_OriginalData) {
@@ -39,14 +57,15 @@ abstract class CRM_CivirulesConditions_Generic_FieldChanged extends CRM_Civirule
   }
 
   /**
-   * This function could be overridden in subclasses to
+   * This method could be overridden in subclasses to
    * transform field data to a certain type
    *
    * E.g. a date field could be transformed to a DataTime object so that
    * the comparison is easier
    *
-   * @param $fieldData
+   * @param mixed $fieldData
    * @return mixed
+   * @access protected
    */
   protected function transformFieldData($fieldData) {
     return $fieldData;
@@ -59,11 +78,19 @@ abstract class CRM_CivirulesConditions_Generic_FieldChanged extends CRM_Civirule
    *
    * @param int $ruleConditionId
    * @return bool|string
+   * @access public
    */
   public function getExtraDataInputUrl($ruleConditionId) {
     return false;
   }
 
+  /**
+   * Method to get the field data
+   *
+   * @param object CRM_Civirules_EventData_EventData $eventData
+   * @return mixed|null
+   * @access protected
+   */
   protected function getFieldData(CRM_Civirules_EventData_EventData $eventData) {
     $entity = $this->getEntity();
     $data = $eventData->getEntityData($entity);
@@ -74,6 +101,13 @@ abstract class CRM_CivirulesConditions_Generic_FieldChanged extends CRM_Civirule
     return null;
   }
 
+  /**
+   * Method to get the original field data
+   *
+   * @param object CRM_Civirules_EventData_Interface_OriginalData $eventData
+   * @return mixed|null
+   * @access protected
+   */
   protected function getOriginalFieldData(CRM_Civirules_EventData_Interface_OriginalData $eventData) {
     $entity = $this->getEntity();
     if ($eventData->getOriginalEntity() != $entity) {
@@ -92,6 +126,7 @@ abstract class CRM_CivirulesConditions_Generic_FieldChanged extends CRM_Civirule
    * Returns an array with required entity names
    *
    * @return array
+   * @access public
    */
   public function requiredEntities() {
     return array($this->getEntity());

CRM/CivirulesConditions/Generic/ValueComparison.php

@@ -11,7 +11,7 @@ abstract class CRM_CivirulesConditions_Generic_ValueComparison extends CRM_Civir
   private $conditionParams = array();
 
   /**
-   * Function to set the Rule Condition data
+   * Method to set the Rule Condition data
    *
    * @param array $ruleCondition
    * @access public
@@ -72,7 +72,7 @@ abstract class CRM_CivirulesConditions_Generic_ValueComparison extends CRM_Civir
   }
 
   /**
-   * Mandatory function to return if the condition is valid
+   * Mandatory method to return if the condition is valid
    *
    * @param object CRM_Civirules_EventData_EventData $eventData
    * @return bool
@@ -86,25 +86,58 @@ abstract class CRM_CivirulesConditions_Generic_ValueComparison extends CRM_Civir
     return $this->compare($value, $compareValue, $this->getOperator());
   }
 
+  /**
+   * Method to compare data
+   *
+   * @param mixed $leftValue
+   * @param mixed $rightValue
+   * @param string $operator
+   * @return bool
+   * @access protected
+   */
   protected function compare($leftValue, $rightValue, $operator) {
     switch ($operator) {
       case '=':
-        return ($leftValue == $rightValue) ? true : false;
+        if ($leftValue == $rightValue) {
+          return true;
+        } else {
+          return false;
+        }
         break;
       case '>':
-        return ($leftValue > $rightValue) ? true : false;
+        if ($leftValue > $rightValue) {
+          return true;
+        } else {
+          return false;
+        }
         break;
       case '<':
-        return ($leftValue < $rightValue) ? true : false;
+        if ($leftValue < $rightValue) {
+          return true;
+        } else {
+          return false;
+        }
         break;
       case '>=':
-        return ($leftValue >= $rightValue) ? true : false;
+        if ($leftValue >= $rightValue) {
+          return true;
+        } else {
+          return false;
+        }
         break;
       case '<=':
-        return ($leftValue <= $rightValue) ? true : false;
+        if ($leftValue <= $rightValue) {
+          return true;
+        } else {
+          false;
+        }
         break;
       case '!=':
-        return ($leftValue != $rightValue) ? true : false;
+        if ($leftValue != $rightValue) {
+          return true;
+        } else {
+          return false;
+        }
         break;
       default:
         return false;