Skip to content

Commit

Permalink
Merge pull request #12 from zumba/dry-cleanup
Browse files Browse the repository at this point in the history
Cleaning up comments, help with DRY, also improve complexity some
  • Loading branch information
jrbasso committed Apr 29, 2016
2 parents c074299 + 9b4fdc6 commit 94f3364
Show file tree
Hide file tree
Showing 3 changed files with 63 additions and 129 deletions.
22 changes: 11 additions & 11 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ $event->userProperties = [
// This works just like you would expect: it will reset what is already there.
// Note that prior to anything being set, $event->userProperties will be null, not an empty array
```
You can find more information about how the Event object works below in the [Events]
You can find more information about how the Event object works below in the [Events](#events) section.
### Use-Case: Events for Many Users
In situations where you will be sending many Amplitude events for different users, you can actually add the user properties on the event object itself as we covered in the previous section. In fact, everything can be set on the event object itself except for the API Key.

Expand Down Expand Up @@ -181,7 +181,7 @@ See the next section for more details about what you can do with the `Event` obj

# Events

We have made the library very flexible, in terms of giving you options for how to set up the event to be sent to Amplitude. Use the method that best suites your own preferences and project needs.
This library is very flexible, in terms of giving you options for how to set up the event to be sent to Amplitude. Use the method that best suites your own preferences and project needs.

## Just Send It!

Expand All @@ -204,7 +204,7 @@ You have the option to use an event object to set up the event, if this is more
// Get the next event that will be queued or sent:
$event = \Zumba\Amplitude\Amplitude::getInstance()->event();

// Set up the even there, by setting properties...
// Set up the event here, by setting properties...
$event->eventType = 'EVENT-NAME';

// Queue or send the event - since we got the event using the event method, it will be the one used on the next
Expand Down Expand Up @@ -321,7 +321,7 @@ foreach ($eventFactory->getEvents() as $event) {
For times that you just want to quickly set some properties on the next event that will be queued or sent, but aren't ready to actually send or queue the event yet, you can pass in an array of properties into the `$amplitude->event()` method.

```php
// Convinience way to quickly add properties to an event, just pass in array of properties to the event method:
// Convenience way to quickly add properties to an event, just pass in array of properties to the event method:
\Zumba\Zumba\Amplitude::getInstance()->event(
[
'eventProp' => 'Event Value',
Expand All @@ -330,7 +330,7 @@ For times that you just want to quickly set some properties on the next event th
]
);

// The above is equivalent of:
// The above is equivalent to:
$event = \Zumba\Zumba\Amplitude::getInstance()->event();
$event->set(
[
Expand All @@ -346,16 +346,16 @@ $event->set(
The difference?

* Both:
* Require the eventType to be set and non-empty.
* Require the `eventType` to be set and non-empty.
* `logEvent()`:
* Requires API Key to be set, will throw exception if not set yet.
* Requires either userId or deviceId to be set, will throw exception if not set either on the amplitude instance, or on the event itself.
* Requires either `userId` or `deviceId` to be set, will throw exception if not set either on the amplitude instance, or on the event itself.
* Always sends the event at the time it is called, assuming requirements met.
* `queueEvent()`:
* Does NOT require API key to be set first.
* Does NOT require userId or deviceId to be set first.
* If either of those are not set, OR if there are still un-sent events in the queue, it will add the event to an internal queue. This queue does not persist across page loads, if any remain in the queue they are lost if not send during that page load.
* If those requirements ARE set, and there is nothing in the queue, it sends immediately. So if you have already initialized Amplitude, set the API key and the userId or deviceId, when you call `queueEvent()` it will behave exactly the same as calling `logEvent()` would.
* Does **not** require API key to be set ***first***.
* Does **not** require `userId` or `deviceId` to be set ***first***.
* If either of those are not set, or if there are still un-sent events in the queue, it will add the event to an internal queue. This queue does not persist across page loads, if any remain in the queue they are lost if not send during that page load.
* If those requirements **are** set, and there is nothing in the queue, it sends immediately. So if you have already initialized Amplitude, set the API key and the `userId` or `deviceId`, when you call `queueEvent()` it will behave exactly the same as calling `logEvent()` would.
* If you do use this, immediately after initializing Amplitude (setting the API key and either the `userId` or `deviceId` in amplitude), be sure to call `$amplitude->logQueuedEvents()` to send any events that are on the queue. If nothing is on the queue, no worries, nothing happens.

Why would you ever use `logEvent()` instead of `queueEvent()`? The use case for that is when sending events for multiple users, in such a way that you are initializing the data then sending right away. Using `logEvent()` in that case you would catch right away if something is not initialized right away (it will throw a logic exception), instead of "quietly" starting up a queue that you may never deal with if not expecting there to be one.
Expand Down
125 changes: 52 additions & 73 deletions src/Amplitude.php
Original file line number Diff line number Diff line change
Expand Up @@ -72,24 +72,19 @@ class Amplitude
* a new object directly.
*
* Useful if want to possibly send multiple events for the same user in a single page load, or even keep track
* of multiple named instances, each could track to it's own api key and/or user.
*
* Example:
* <code>
* // In user initialization section of app...
* Zumba\Amplitude\Amplitude::getInstance()->init('APIKEY','[email protected]')
* ->setUserProperties(['name' => 'Johnny 5'])
* ->logQueuedEvents();
*
* // Elsewhere in your app, this could happen before OR after the above initialization...
* // See queueEvent() for more info
* Zumba\Amplitude\Amplitude::getInstance()->queueEvent(
* 'VEHICLE CREATED EVENT',
* ['wheels' => '2', 'name' => 'bicycle']
* );
* </code>
*
* @param string $instanceName Optional, can use to maintain multiple singleton instances of amplitude
* of multiple named instances, each could track to it's own api key and/or user/device ID.
*
* Each instance maintains it's own:
* - API Key
* - User ID
* - Device ID
* - User Properties
* - Event Queue (if events are queued before the amplitude instance is initialized)
* - Event object - for the next event that will be sent or queued
* - Opt out status
*
* @param string $instanceName Optional, can use to maintain multiple singleton instances of amplitude, each with
* it's own API key set
* @return \Zumba\Amplitude\Amplitude
*/
public static function getInstance($instanceName = 'default')
Expand All @@ -108,7 +103,7 @@ public static function getInstance($instanceName = 'default')
public function __construct($apiKey = null)
{
if (!empty($apiKey)) {
$this->apiKey = $apiKey;
$this->apiKey = (string)$apiKey;
}
}

Expand All @@ -123,7 +118,7 @@ public function __construct($apiKey = null)
*/
public function init($apiKey, $userId = null)
{
$this->apiKey = $apiKey;
$this->apiKey = (string)$apiKey;
if ($userId !== null) {
$this->setUserId($userId);
}
Expand Down Expand Up @@ -165,28 +160,8 @@ public function resetQueue()
/**
* Gets the event that will be used for the next event logged by call to logEvent() or queueEvent()
*
* If this style suites your project, you can do something like this:
*
* <code>
* $event = $amplitude->event();
* // Set up event here...
* $event->set('event_type', 'EVENT TYPE')
* ->set('custom property', 'Some Value');
*
* // Calling this will log the event we just set up
* $amplitude->logEvent();
* </code>
*
* That would be the equivelent of this:
* <code>
* $amplitude->logEvent('EVENT TYPE', ['custom property' => 'Some Value']);
* </code>
*
* Note that either use above requires amplitude to be set up first with all the required things set (like api
* key and either user ID or device ID). You can replace logEvent with queueEvent if you may need to run it before
* amplitude is fully initialized.
*
* You can also pass in an event to set as the next event to run.
* You can also pass in an event or array of event properties. If you pass in an event, it will be set as the
* event to be used for the next call to queueEvent() or logEvent()
*
* @param null|array|\Zumba\Amplitude\Event Can pass in an event to set as the next event to run, or array to set
* properties on that event
Expand All @@ -210,8 +185,6 @@ public function event($event = null)
/**
* Resets the event currently in the process of being set up (what is returned by event())
*
* This could end up clearing user data set.
*
* @return \Zumba\Amplitude\Amplitude
*/
public function resetEvent()
Expand All @@ -224,10 +197,10 @@ public function resetEvent()
* Log an event immediately
*
* Requires amplitude is already initialized and user ID or device ID is set. If need to wait until amplitude
* is initialized, use queueEvent method instead.
* is initialized, use queueEvent() method instead.
*
* Can either pass in information to be logged, or can set up the event using OOP, see the event() method for more
* information
* Can either pass in information to be logged, or can set up the Event object before hand, see the event()
* method for more information
*
* @param string $eventType Required if not set on event object prior to calling this
* @param array $eventProperties Optional, properties to set on event
Expand All @@ -244,22 +217,10 @@ public function logEvent($eventType = '', array $eventProperties = [])
throw new \LogicException(static::EXCEPTION_MSG_NO_API_KEY);
}
$event = $this->event();
if (!empty($eventProperties)) {
$event->set($eventProperties);
}
if (!empty($eventType)) {
$event->eventType = $eventType;
}
if (!empty($this->userId) && empty($event->userId)) {
$event->userId = $this->userId;
}
if (!empty($this->deviceId) && empty($event->deviceId)) {
$event->deviceId = $this->deviceId;
}
if (!empty($this->userProperties)) {
$event->setUserProperties($this->userProperties);
$this->resetUserProperties();
}
$event->set($eventProperties);
$event->eventType = $eventType ?: $event->eventType;
// Set the persistent options on the event
$this->setPersistentEventData();

if (empty($event->eventType)) {
throw new \LogicException(static::EXCEPTION_MSG_NO_EVENT_TYPE);
Expand All @@ -276,6 +237,26 @@ public function logEvent($eventType = '', array $eventProperties = [])
return $this;
}

/**
* Set the persistent data on the event object
*
* @return void
*/
protected function setPersistentEventData()
{
$event = $this->event();
if (!empty($this->userId)) {
$event->userId = $this->userId;
}
if (!empty($this->deviceId)) {
$event->deviceId = $this->deviceId;
}
if (!empty($this->userProperties)) {
$event->setUserProperties($this->userProperties);
$this->resetUserProperties();
}
}

/**
* Log or queue the event, depending on if amplitude instance is already set up or not
*
Expand All @@ -285,7 +266,9 @@ public function logEvent($eventType = '', array $eventProperties = [])
* event to be logged later (during same page load).
*
* If the API key, and either user ID or device ID are already set in the amplitude instance, and there is not
* already events in the queue that have not been run, this will log the event immediately.
* already events in the queue that have not been run, this will log the event immediately. Note that having
* the userId or deviceId set on the event itself does not affect if it queues the event or not, only if set on
* the Amplitude instance.
*
* Otherwise it will queue the event, and will be run after the amplitude instance is initialized and
* logQueuedEvents() method is run
Expand All @@ -302,9 +285,7 @@ public function queueEvent($eventType = '', array $eventProperties = [])
}
$event = $this->event();
$event->set($eventProperties);
if (!empty($eventType)) {
$event->eventType = $eventType;
}
$event->eventType = $eventType ?: $event->eventType;

// Sanity checking
if (empty($event->eventType)) {
Expand All @@ -323,30 +304,28 @@ public function queueEvent($eventType = '', array $eventProperties = [])
/**
* Set the user ID for future events logged
*
* Note that setting the user ID directly on an individual Event object will take precedence over one set with this
* method.
* Any set with this will take precedence over any set on the Event object
*
* @param string $userId
* @return \Zumba\Amplitude\Amplitude
*/
public function setUserId($userId)
{
$this->userId = $userId;
$this->userId = (string)$userId;
return $this;
}

/**
* Set the device ID for future events logged
*
* Note that setting the device ID directly on an individual Event object will take precedence over one set with
* this method.
* Any set with this will take precedence over any set on the Event object
*
* @param string $deviceId
* @return \Zumba\Amplitude\Amplitude
*/
public function setDeviceId($deviceId)
{
$this->deviceId = $deviceId;
$this->deviceId = (string)$deviceId;
return $this;
}

Expand Down
45 changes: 0 additions & 45 deletions src/Event.php
Original file line number Diff line number Diff line change
Expand Up @@ -124,21 +124,6 @@ public function setUserProperties(array $userProperties)
*
* It also accepts an array of key => value pairs for the first argument, to pass in an array of properties to set.
*
* All of these are equivelent, and any of these would set the event property "device_brand" to "HTC":
*
* <code>
* $event->set('device_brand', 'HTC');
* $event->set('deviceBrand', 'HTC');
* // Object magic methods
* $event->device_brand = 'HTC';
* $event->deviceBrand = 'HTC';
* // setting array
* $event->set(['device_brand' => 'HTC']);
* $event->set(['deviceBrand' => 'HTC']);
* </code>
*
* All of the above are equivelent, use whatever is most appropriate for your project / situation.
*
* Note that only built-in event properties are normalized to match the built-in name. Custom properties that get
* set in event_properties are not normalized. Meaning if you use a camelcase name, name with spaces in it, etc,
* it will use that name as-is without attempting to normalize.
Expand Down Expand Up @@ -186,16 +171,6 @@ public function set($name, $value = null)
* As with the set() method, for built-in event properties, can use camelcase OR underscore and either one will
* work. This is not the case for custom event properties however.
*
* For example, any of these calls will get the value of device_brand:
*
* <code>
* $event->get('device_brand');
* $event->get('deviceBrand');
* // Magic methods work too:
* $event->device_brand;
* $event->deviceBrand;
* </code>
*
* If no value found, returns null.
*
* @param string $name
Expand All @@ -218,16 +193,6 @@ public function get($name)
* As with the set() method, for built-in event properties, can use camelcase OR underscore and either one will
* work. This is not the case for custom event properties however.
*
* For example, any of these calls will unset the main built-in property device_brand:
*
* <code>
* $event->unsetProperty('device_brand');
* $event->unsetProperty('deviceBrand');
* // Magic methods work too:
* unset($event->device_brand);
* unset($event->deviceBrand);
* </code>
*
* @param string $name
* @return \Zumba\Amplitude\Event
*/
Expand All @@ -248,16 +213,6 @@ public function unsetProperty($name)
* As with the set() method, for built-in event properties, can use camelcase OR underscore and either one will
* work. This is not the case for custom event properties however.
*
* For example, any of these calls will check if the built in property for device_brand is set:
*
* <code>
* $event->isPropertySet('device_brand');
* $event->isPropertySet('deviceBrand');
* // Magic methods work too:
* isset($event->device_brand);
* isset($event->deviceBrand);
* </code>
*
* @param string $name
* @return \Zumba\Amplitude\Event
*/
Expand Down

0 comments on commit 94f3364

Please sign in to comment.