peussen / tripolis-api

PHP Library to connect to the Tripolis SOAP API
1 stars 2 forks source link

Just pass an Array as body instead of variables - v2.0 suggestion #2

Open jmslbam opened 7 years ago

jmslbam commented 7 years ago

I had a bunch of missing arguments ( smartGroupId & scheduleAt) for methodes like public function publishEmail($eId, $contactGroupId, $mailJobTags = array()) https://github.com/martybel/tripolis-api/blob/bb7c0be039f3ba2bb6f002a739b984cbf5b40519/src/Service/PublishingService.php#L33

So I changed it to just pass a $body array to the method. Less code and more flexibility. Example: https://github.com/level-level/tripolis-api/commit/d2d20010028bb7700d4935e20f8976c5a22eccdb

This will break backwards compatibility and this should be consistent throughout the whole project, so no PR for now 💃

peussen commented 7 years ago

Just passing an array is a bit tricky.. there are some entries that should be set always, which i either have to check, and throw an error for then, or have to pass and leave it up to the developer. Neither seem like a real friendly solution.

How about adding an extra parameter which allows you to set all options when you want them (basically create your own body), and make sure the required entries are filled in the methods.

I created this as a commit in a seperate branch https://github.com/martybel/tripolis-api/commit/cf798a495735f7fb9eef8c7ce997a6ee5bf1c889

Since i lost my access to tripolis i can't check it, so perhaps you can check it out and perhaps see if this will work?

jmslbam commented 7 years ago

v1 solution

I haven't tested this code, but to simplify your work through something like this?

public function publishTransactionalEmail( $contactId, $emailId, $mailJobTags = array(), $extra_body = array() ){
    $defaults = array(
        'contactId'     => $contactId,
        'directEmailId' => $emailId,
        'mailJobTagIds' => array(
            'mailJobTagId' => $mailJobTags
        )
    );

    $body = array_merge( $defaults, $extra_body );

    return $this->invoke( __FUNCTION__, $body );
}
jmslbam commented 7 years ago

v2

The previous solution is O.K. with the $extra_body argument. But if one still want to remove all the other args, and just have one $body array, this could be a solution. It will be more flexible, but will break BC.

If you want to ensure that some arguments are mandatory this can be implemented:

<?php
public function publishTransactionalEmail( $body = array() ){

    if( ! self::validateKeys( ['contactId', 'directEmailId', 'mailJobTagIds' ], $body ) ){
        // trigger / log error 
    }

    return $this->invoke( __FUNCTION__, $body );
}

/**
 * Helper function the easy validate key values in an Array
 * Check if the key exists and if it's not empty.
 */
public static function validateKeys( $keys, $array ){
    foreach( $keys as $key ){
        if( !array_key_exists($key, $array) || empty( $array[ $key ]) ) {
            // Also log error
            return false;
        }
    }

    return true;
}