3
\$\begingroup\$

I am experimenting with the Five9 API and setting up a PHP class to deal with the addRecordstoList method.

The main issue this class was meant to solve over the traditional function found in their documentation is that the function must map the index of the array to the correct field. For instance, column 1 is populated by value of index 0 of the array. This would cause an issue when the lead array sent as a parameter has keys in a different order or includes optional fields. It throws the indexes off and information on the records list is out of order when sent over.

I would like some input on the class and how to make it faster and allow to sendtoFive9 function to be looped without overriding the array.

    //how the columns are mapped in the example provided by five9
    $listUpdateSettings = array ( "fieldsMapping" => array (
     array ( "columnNumber" => '1', "fieldName" =>
    "number1", "key" => true ),
     array ( "columnNumber" => '2', "fieldName" => "first_
    name", "key" => false ),
     array ( "columnNumber" => '3', "fieldName" => "last_
    name", "key" => false) );
    
    class five9{
    
      //the fields mapped to five9
      //initialize and empty array so there are no errors with array_push
      private static $listUpdateSettings = array("fieldsMapping" => array());
    
      //maps field to five9 columns
      protected function mapField($index, $field, $key = false){
        //map the field to five9. Index must start at 1 not 0
        //mapping fields this way allows flexibility in which order the parameter has its keys
        array_push( self::$listUpdateSettings["fieldsMapping"], array( "columnNumber" => $index + 1, "fieldName" => $field, "key" => $key ));
      }
    
      //returns data array after being scrubbed
      protected function checkArray( array $lead){
    
        //data sent to five9
        $data = array();
    
        //counter
        $i = 0;
    
        //five9 requires the $field names outlined below
        foreach($lead as $field => $value){
          //make sure first_name is 3 characters or more
          if( ($field == 'first_name' && strlen($field) > 3)){
            //add field to array
            $data[$i] = $value;
            //map the field in five9
            self::mapField($i, $field);
          }
          //make sure last_name is 3 characters or more
          if($field == 'last_name' && strlen($field) > 3 ){
            //add field to array
            $data[$i] = $value;
            //map the field in five9
            self::mapField($i, $field);
          }
          //if the field is a phone number
          if( $field == 'number1' ){
            //add field to array
            $data[$i] = preg_replace("/[^0-9]/", "", $value);
            //map the field in five9
            //this was they key for my instance
            self::mapField($i, $field, true);
          }
          //if the field is a phone number
          if( $field == 'number2' ){
            //add field to array
            $data[$i] = preg_replace("/[^0-9]/", "", $value);
            //setup column mapping in five9
            self::mapField($i, $field);
          }
          //make sure the state is only two characters
          if($field == 'state' && strlen($value) <= 2){
            //add field to array
            $data[$i] = $value;
            //setup column mapping in five9
            self::mapField($i, $field);
          }
          //make sure the zip is only 5 characters
          if($field == 'zip' && strlen($value) == 5){
            //add field to array
            $data[$i] = $value;
            //setup column mapping in five9
            self::mapField($i, $field);
          }
          //make sure memberid is an int
          if($field == 'member_id' && is_numeric($value)){
            //add field to array
            $data[$i] = $value;
            //setup column mapping in five9
            self::mapField($i, $field);
          }
          //increase the counter
          $i++;
        }
        //return the data array that is constructed
        return $data;
    
      }
    
      static function sendToFive9(array $lead ){
    
        //the conctructed array
        $data = self::checkArray($lead);
    
        //if the fields sent are all correct both arrays are the same size
        if(sizeof($lead) === sizeof($data) ){
    
            // Import the WSDL and authenticate the user.-----------------------------
            $wsdl_five9 = "https://api.five9.com/wsadmin/v2/AdminWebService?wsdl&user=$username";
    
            //try to authenticate with five9
            try{
                $soap_options = array( 'login'    => '$username',
                                       'password' => '$password',
                                       'trace' => true );
    
                $client_five9 = new SoapClient( $wsdl_five9 , $soap_options );
            }//if errors occur add the message to the response array
            catch (Exception $e){
                $error_message = $e->getMessage();
                $response['error'] = $error_message;
            }
    
    
            //settings required by five9
            self::$listUpdateSettings["skipHeaderLine"] = false;
            self::$listUpdateSettings["cleanListBeforeUpdate"] = false;
            self::$listUpdateSettings["crmAddMode"] = 'ADD_NEW';
            self::$listUpdateSettings["crmUpdateMode"] = 'UPDATE_SOLE_MATCHES';
            self::$listUpdateSettings["listAddMode"] = 'ADD_IF_SOLE_CRM_MATCH';
    
            //the default list for all new leads
            $list = "test";
    
            //prepare the query used to add the record to five9
            $query = array ( 'listName' => "$list",
                             'listUpdateSettings' => self::$listUpdateSettings,
                             'record' => $data );
    
            //get the result from running the query
            //this will return an object
            $result = $client_five9->AddRecordToList($query);
    
            //get the array of variables within the results object
            $variables = get_object_vars($result);
    
            //get the array of varaibles within the return array of objects
            $resp = get_object_vars($variables['return']);
    
            //if there was an error adding the record
            if($resp['failureMessage'] != ""){
              $response['errors'] = $resp['failureMessage'];
            }
           //if it was successful either adding or updating
            if($resp['crmRecordsUpdated'] == 1 || $resp['crmRecordsInserted'] == 1){
              $response['success'] = true;
              //reset the settings array so this function can be looped without passing the lists back and forth
              self::$listUpdateSettings = array("fieldsMapping" => array());
            }
    
        }//end if
        else{
            //return the differences in the arrays usually caused due to improper names
            $reponse["errors"] = array_diff($lead, $data);
    
        }
    
        return $response;
    
      }//end function
}
\$\endgroup\$
3
  • \$\begingroup\$ For the first couple if statements, should it be checking the length of $value instead of $field? e.g. if( ($field == 'first_name' && strlen($field) > 3)){ \$\endgroup\$ Commented Oct 6, 2017 at 17:33
  • \$\begingroup\$ In terms of readability, I think your functions are too big and it would be more readable if you split it up in multiple functions. Next to that, please adhere to PSR2. Also for readability and performance you should use elseif blocks instead of only ifs. Also, I would suggest using late static bindings: static:: instead of self:: \$\endgroup\$
    – Mazzy
    Commented Oct 8, 2017 at 21:53
  • \$\begingroup\$ @Mazzy thanks for the input will include in the next revision, I updated my answer below with some of the suggestions incorporated. \$\endgroup\$
    – Orlando P.
    Commented Oct 10, 2017 at 13:28

2 Answers 2

1
\$\begingroup\$

Updating my answer after input from everyone. Posted this on Github but I am still working on it.

<?php
/**
 * This class uses five9 API to send records in various ways to a contact list
 * 
 * @link       https://github.com/opolanco23/PHP-Five9-API
 * @since      1.0.0
 *
 * @package    PHP Five9 API
 * @author     Orlando Polanco <[email protected]>
 */
Class addRecordsToFive9{
  /**
     * The username set outside of any function so it can be set
     * by any method required by the client code
   *
     * @since    1.0.0
     * @access   private
     * @var      string    $username    The username used to connect to five9.
     */
   private static $username = "";
   /**
     * The password set outside of any function so it can be set
     * by any method required by the client code
    *
     * @since    1.0.0
     * @access   private
     * @var      string    $password    The username used to connect to five9.
    */
  private static $password = "";
  /**
   * The array consists of keys that must match the name of fields inside your five9 contact field.
   * Each key is used to associate an array of settings to that field
   * supported data types are string, phone, int you may add as needed.
   * @since    1.0.0
   * @access   private
   * @var      array    $fields   (field name within five9) "key"  => array("type of data", min-length, max-length, is_key)
  */
  private static $fields = array(
            "first_name" =>  array( 'type' => "string"  , 'min' =>   3, 'max' => 100, 'is_key' => false ),
            "last_name"  =>  array( 'type' => "string"  , 'min' =>   3, 'max' => 100, 'is_key' => false ),
            "number1"    =>  array( 'type' => "phone"   , 'min' =>  10, 'max' =>  14, 'is_key' => true  ),
            "number2"    =>  array( 'type' => "phone"   , 'min' =>  10, 'max' =>  14, 'is_key' => false ),
            "state"      =>  array( 'type' => "string"  , 'min' =>   2, 'max' =>   2, 'is_key' => false ),
            "zip"        =>  array( 'type' => "int"     , 'min' =>   5, 'max' =>   5, 'is_key' => false ),
            "member_id"  =>  array( 'type' => "int"     , 'min' =>   5, 'max' =>  20, 'is_key' => false ),
  );
  /**
     * Constructor is not used for any processing yet
   * but could be used to Initialize username and password.
   *
     * @since    1.0.0
   * @return   null
    */
  function __construct(){
    /* call to DB or external file to retrieve username and password
     * $credentials = getCredentials();
     * $this->$username = $credentials['username'];
     * $this->$password = $credentials['password'];
    */
  }
  /**
     * Before Sending record to five9 the fields must be mapped with column number and the field name.
     * Names of the fields must also match the names inside of the contact database.
   *
     * @since    1.0.0
     * @param    array         $record          the scrubbed array of lead data.
     * @param    array         $mappedFields    An array of fields being mapped to five9 along with there properties
   * @param    integer       $index           where to start the column count. Some functions require 1 others 0
   * @return   array         $mappedFields    returns an array of arrays consisiting of the mappedFields
    */
  protected function mapFields($record, $fields, $i){
    foreach ($record as $key => $value) {
      //map the field to the five9 system
      $mappedFields[] =  array( "columnNumber" => $i, "fieldName" => $key, "key" => $fields[$key]['is_key'] );
      $i++;
    }
    return $mappedFields;
  }
  /**
     * Before Sending record to five9 the fields must be scrubbed to make sure the fields have the proper names.
     * They also are scrubbed of any uncessary data decided by you or the system
   *
     * @since    1.0.0
     * @param    string        $fields           The array of field names and corresponding properties set for validation
     * @param    boolean       $lead             the array of lead data that was sent from the client code.
   * @return   array         $data             the scrubed array of lead data.
    */
  protected function scrubArray($fields, $lead){
    foreach($lead as $key => $value){
      //if the keys match and the field is the correct size
      if( array_key_exists($key, $fields) && ( strlen($value) >= $fields[$key]['min'] && strlen($value) <= $fields[$key]['max'] ) ):
        if($fields[$key]['type'] == 'string' && is_string($value)){
          $data[$key] = $value;
        }
        if($fields[$key]['type'] == 'phone' ){
          $data[$key] = preg_replace("/[^0-9]/", "", $value);
        }
        if($fields[$key]['type'] == 'int' && is_numeric($value)){
          $data[$key] = $value;
        }
      endif; //end keys match if
    }
    return $data;
  }
  //static function because authentication won't change
  protected static function authenticateMe(){
    // Import the WSDL and authenticate the user.-----------------------------
    $wsdl_five9 = "https://api.five9.com/wsadmin/v2/AdminWebService?wsdl&user=" . self::$username ;
    //try to authenticate with five9
    try{
        $soap_options = array( 'login'    =>  self::$username,
                               'password' =>  self::$password,
                               'trace' => true );
        $client_five9 = new SoapClient( $wsdl_five9 , $soap_options );
        $response['success'] = $client_five9;
    }//if errors occur add the message to the response array
    catch (Exception $e){
        $error_message = $e->getMessage();
        $response['error'] = $error_message;
    }
    return $response;
  }
  //send the record to five9
  function addRecordToList($lead, $list ){
    //the conctructed array
    $data = $this->scrubArray( self::$fields, $lead );
    //if the fields sent are all correct both arrays are the same size
    if(sizeof($lead) === sizeof($data) ){
      $client_five9 = self::authenticateMe();
      if( array_key_exists('success', $client_five9) ){
         //get the Soap Object
         $client = $client_five9['success'];
         //map the fields to five9 with the new ordered array
         $mappedFields = $this->mapFields($data, self::$fields, 1);
         //if the member_id is also included then send it to the memebers list
         if (array_key_exists("member_id" , $data)){
           $list = "members-oep";
         }
         //the mapped fields column number must match the index of the record
         //therefore we must make the associated array and indexed one
         $data = array_values($data);
          //settings required by five9
          $listUpdateSettings["fieldsMapping"] = $mappedFields;
          $listUpdateSettings["skipHeaderLine"] = false;
          $listUpdateSettings["cleanListBeforeUpdate"] = false;
          $listUpdateSettings["crmAddMode"] = 'ADD_NEW';
          $listUpdateSettings["crmUpdateMode"] = 'UPDATE_SOLE_MATCHES';
          $listUpdateSettings["listAddMode"] = 'ADD_IF_SOLE_CRM_MATCH';
          //prepare the query used to add the record to five9
          $query = array ( 'listName' => "$list",
                           'listUpdateSettings' => $listUpdateSettings,
                           'record' => $data );
         //try to add the record the five9 system
         try{
            //get the result from running the query
            //this will return an object
            $result = $client->AddRecordToList($query);
            //get the array of variables within the results object
            $resp = get_object_vars($result->return);
            //if there was an error adding the record
            if($resp['failureMessage'] != ""){
              $response['errors'] = $resp['failureMessage'];
            }
           //if it was successful either adding or updating
            if($resp['crmRecordsUpdated'] == 1 || $resp['crmRecordsInserted'] == 1){
              $response['success'] = true;
            }
          }//adding failed respond with error
          catch (Exception $e){
               //get the error message
               $error_message = $e->getMessage();
               //add the error message to the response array
               $response['error'] = $error_message;
          }
       }//end arraykey if
    }//end sizeof if
    else{
        //return the differences in the arrays usually caused due to improper names
        $response["errors"] = array_diff($lead, $data);
    }
    return $response;
  }//end function
  //more to come
  protected function addToListCsv(){
  }

  //more to come
  protected function createList(){
  }
}
?>
\$\endgroup\$
1
  • \$\begingroup\$ @Orlando P. Please read How do I write a good answer?: "Every answer must make at least one insightful observation about the code in the question." However, this post is merely an update of the code. It would be advisable to edit the question and add the updated code. \$\endgroup\$ Commented Nov 16, 2017 at 0:42
0
\$\begingroup\$

Feedback

The class looks like it is organized well for the most part- the methods generally handle a single task. However, some of those methods are a bit lengthy so could be broken up into smaller sub-methods. The updated code posted in the answer has methods with shorter lengths, except for the principle method addRecordToList().

In the original code there is a variable $listUpdateSettings outside the class. Was that just left over from debugging or other code?

Suggestions

Below are some options for cleaning up the code and possibly making it more sustainable as the number of fields supported grows.

Instance variables instead of passing parameters around

Because you are defining a class, one could take advantage of instance variables. $data and mappedFields can both be declared as arrays. Unless they need to be accessed outside the code within the class, set the scope to private (or protected if a sub-class needed to utilize them).

class five9{

    private $data = array();
    private $mappedFields = array();

Then just reference those variables when needed:

function MapField($index, $field, $key){
    //map the field to the five9 system
    $this->mappedFields[] =  array( "columnNumber" => $index + 1, "fieldName" => $field, "key" => $key );
}
....
function SendtoFive9($lead){

    //the conctructed array
    $this->ScrubArray($lead);
    if(sizeof($lead) === sizeof($this->data) ){

        ...
        $listUpdateSettings["fieldsMapping"] = $this->mappedFields;

Validating data in ScrubArray()

There are various options for shortening the code in ScrubArray(). Two such options are outlined below, though other options are possible as well. The first approach aims for the fewest number of lines of code (while possibly sacrificing speed), while the second breaks the existing validation out into separate methods and calls them dynamically.

Validation with Regular expressions for all fields

One option is to define regular expressions for each field that needs to be validated. Bear in mind that performing a regular expression match on a string just to check the length will typically be slower than using strlen() but this way you can look for validation rules and ensure the values match the format.

In the code below, the ReflectionClass is used to look up the constants in the class definition and if found, will use preg_match() to ensure the values conform to the field's validation rules. You could also look into using filter_var() with FILTER_VALIDATE_REGEXP

const VALIDATION_FIRST_NAME = '#^.{3,}$#';
const VALIDATION_LAST_NAME = '#^.{3,}$#';
const VALIDATION_STATE = '#^\w{0,2}$#';
const VALIDATION_ZIP = '#^\d{5}$#';
const VALIDATION_MEMBER_ID = '#^\d+$#';
const SANITAION_NUMBER1 = '#[^\d]#';
const SANITAION_NUMBER2 = '#[^\d]#';
function ScrubArray($lead){
    $fields = array('first_name', 'last_name', 'number1', 'number2', 'state', 'zip', 'member_id');

    $reflection = new ReflectionClass($this);
    $constants = $reflection->getConstants();
    foreach(array_keys($lead) as $index => $field) {
        if (in_array($field, $fields)) {
            $constantKey = 'VALIDATION_'.strtoupper($field);
            $value = $lead[$field];
            $key = ($field == 'number1'); //@TODO: decide with constant, method, etc.
            if (!$constants[$constantKey] || preg_match($constants[$constantKey], $value)) {
                $this->MapField($index, $field, $key);
                $sanitationKey = 'SANITATION_'.strtoupper($field);
                if ($constants[$sanitationKey]) {
                    $value = preg_replace($sanitationKey, '', $value);
                }
                $this->data[] = $value;
            }
        }
    }
}

Validation with separate methods

To keep the string length validation using strlen(), you could define separate methods and call those for each field. For example, the first_name and last_name fields could both be validated by the same method:

/**
* Validate that a name field has more than 3 characters
* @return string | bool 
*/
function validateNameField($value) {
    if (strlen($value) > 3) {
        return $value;
    }
    return false;
}

And similarly for the other fields:

/**
 * Validate a number field - merely removes any non-numeric characters
 * @return string 
 */
function validateNumberField($value) {
  return preg_replace("/[^0-9]/", "", $value);
}
/**
 * Validate that a statefield has no more than 2 characters
 * @return string | bool 
 */
function validateStateField($value) {
  if (strlen($value) <= 2) {
    return $value;
  }
  return false;
}
/**
 * Validate that a ZIP field has exactly 5 characters
 * @return string | bool 
 */
function validateZipField($value) {
  if (strlen($value) == 5) {
    return $value;
  }
  return false;
}
/**
 * Validate that a member ID field is numeric
 * @return string | bool 
 */
function validateMemberIdField($value) {
  if (is_numeric($value)) {
    return $value;
  }
  return false;
}

Then those methods could be mapped to the field names, perhaps in a helper method:

function getFieldValidationMethodMapping() {
      return array(
        'first_name' => 'validateNameField', 
        'last_name' => 'validateNameField', 
        'number1' => 'validateNumberField', 
        'number2' => 'validateNumberField',
        'state' => 'validateStateField',
        'zip' => 'validateZipField', 
        'member_id' => 'validateMemberIdField'
      );
}

Then the ScrubArray method can look up the method names based on the field name and call them dynamically:

function ScrubArray($lead){
  $fieldMapping = $this->getFieldValidationMethodMapping();
  foreach(array_keys($lead) as $index => $field) {
    if (array_key_exists($field, $fieldMapping));
      if (method_exists($this, $fieldMapping[$field])) {
          $validationValue = $this->{$fieldMapping[$field]}($lead[$field]);
          if ($validationValue !== false) {
            $key = ($field == 'number1'); //@TODO: decide with constant, method, etc.
            $this->MapField($index, $field, $key);
            $this->data[] = $validationValue;
          }
      }
  }
}

Unit Test

Perhaps you already have a unit test, but if not (or even if you do), you might find this unit test useful. Note that in my testing I actually made those instance variables $data and $mappedFields public for ease in testing, but if you take my advice above to make those fields private then you may want to make methods to get the values in those instances variables, since outside code could not directly access them.

Update

You posted your updated code and asked for it to be reviewed. I think it looks pretty good. My only suggestion is to remove the formal parameter $i from the method mapFields(), since it is only called in one place with the value 1. That value can be assigned to 1 at the start of the method, and then incremented using the post-increment operator whenever after it is used:

protected function mapFields($record, $fields){
    $i = 1;
    foreach ($record as $key => $value) {
      //map the field to the five9 system
      $mappedFields[] =  array( "columnNumber" => $i++, "fieldName" => $key, "key" => $fields[$key]['is_key'] );
    }
    return $mappedFields;
}

If you wanted to avoid the possible undefined variable $mappedFields you could check to make sure count($mappedFields) > 0 evaluates to true.

\$\endgroup\$
3
  • \$\begingroup\$ Hey I actually rewrote this further and put it on github, I had forgotten to update it on here. I will review your answer and incorporate some of the tips github.com/opolanco23/PHP-Five9-API/blob/master/… \$\endgroup\$
    – Orlando P.
    Commented Oct 7, 2017 at 23:56
  • \$\begingroup\$ Cool - I expanded my answer, providing another approach to re-writing the ScrubArray() method \$\endgroup\$ Commented Oct 9, 2017 at 20:44
  • \$\begingroup\$ awesome explanation. Thanks for the input really appreciate it. If you could review the updated answer for any improvements I would also I appreciate it. \$\endgroup\$
    – Orlando P.
    Commented Oct 10, 2017 at 13:30

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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