I am trying to improve the below switch statement. What's happening here is that the code is called multiple times based on an x amount of tokens found, so the below code runs once per token.
If the $post->ID
is not found then a notification is sent to that token and the id gets added in the database.
This works however at some point it's stopping after around 40% of tokens checked presumably because the ID is found? Since I am on wordpress, I used the update_option
to store the id in a table but perhaps an alternative approach can be used?
$os = $this->os;
switch ($os) {
case "iOS":
$iOS_pastPushSavedID = get_option( 'iOS_pastPushSavedID', $default = false);
if($post->ID != $iOS_pastPushSavedID) {
update_option( 'iOS_pastPushSavedID', $post->ID, no);
$sendPush = true;
//$title = ($os . '_New Push = ' . ' storedID: ' . $iOS_pastPushSavedID . ' / postID: ' . $post->ID);
} else {
//$title = ($os . '_Duplicate Push = ' . ' storedID: ' . $iOS_pastPushSavedID . ' / postID: ' . $post->ID);
$sendPush = false;
}
break;
case "Android":
$android_pastPushSavedID = get_option( 'android_pastPushSavedID', $default = false);
if($post->ID != $android_pastPushSavedID) {
//$title = ($os . '_New Push = ' . ' storedID: ' . $android_pastPushSavedID . ' / postID: ' . $post->ID);
update_option( 'android_pastPushSavedID', $post->ID, no);
$sendPush = true;
} else {
//$title = ($os . '_Duplicate Push = ' . ' storedID: ' . $android_pastPushSavedID . ' / postID: ' . $post->ID);
$sendPush = false;
}
break;
case "Fire OS":
$fireos_pastPushSavedID = get_option( 'fireos_pastPushSavedID', $default = false);
if($post->ID != $fireos_pastPushSavedID) {
//$title = ($os . '_New Push = ' . ' storedID: ' . $fireos_pastPushSavedID . ' / postID: ' . $post->ID);
update_option( 'fireos_pastPushSavedID', $post->ID, no);
$sendPush = true;
} else {
//$title = ($os . '_Duplicate Push = ' . ' storedID: ' . $fireos_pastPushSavedID . ' / postID: ' . $post->ID);
$sendPush = false;
}
break;
case "Safari":
$safari_pastPushSavedID = get_option( 'safari_pastPushSavedID', $default = false);
if($post->ID != $safari_pastPushSavedID) {
//$title = ($os . '_New Push = ' . ' storedID: ' . $safari_pastPushSavedID . ' / postID: ' . $post->ID);
update_option( 'safari_pastPushSavedID', $post->ID, no);
$sendPush = true;
} else {
//$title = ($os . '_Duplicate Push = ' . ' storedID: ' . $safari_pastPushSavedID . ' / postID: ' . $post->ID);
$sendPush = false;
}
break;
case "Chrome":
$chrome_pastPushSavedID = get_option( 'chrome_pastPushSavedID', $default = false);
if($post->ID != $chrome_pastPushSavedID) {
//$title = ($os . '_New Push = ' . ' storedID: ' . $chrome_pastPushSavedID . ' / postID: ' . $post->ID);
update_option( 'chrome_pastPushSavedID', $post->ID, no);
$sendPush = true;
} else {
//$title = ($os . '_Duplicate Push = ' . ' storedID: ' . $chrome_pastPushSavedID . ' / postID: ' . $post->ID);
$sendPush = false;
}
break;
case "Firefox":
$firefox_pastPushSavedID = get_option( 'firefox_pastPushSavedID', $default = false);
if($post->ID != $firefox_pastPushSavedID) {
//$title = ($os . '_New Push = ' . ' storedID: ' . $firefox_pastPushSavedID . ' / postID: ' . $post->ID);
update_option( 'firefox_pastPushSavedID', $post->ID, no);
$sendPush = true;
} else {
//$title = ($os . '_Duplicate Push = ' . ' storedID: ' . $firefox_pastPushSavedID . ' / postID: ' . $post->ID);
$sendPush = false;
}
break;
default:
$sendPush = false;
}
Unless I am misunderstanding your process, this is a very DRY / concise way to do it without the verbose switch/case
block:
$os_opts=[
'iOS'=>'iOS',
'Android'=>'android',
'Fire OS'=>'fireos',
'Safari'=>'safari',
'Chrome'=>'chrome',
'Firefox'=>'firefox'
];
$os=$this->os;
$sendPush=false;
if(isset($os_opts[$os])){ // deny empty and invalid options
$os_opt="{$os_opts[$os]}_pastPushSavedID"; // build string for next two functions
if($post->ID!=get_option($os_opt,$default=false)){
update_option($os_opt,$post->ID,no);
$sendPush = true;
}
}
The $os_opts
array has keys that match $os
, and values that work with get_option()
& update_option()
. This will largely reduce code length, and make future modifications very easy to do.
Since the get_option()
result is only used once, it doesn't make sense to declare it as a variable; just use it in the if condition.
The first parameter of get_option()
and update_option()
always end with the same substring. I makes sense to prepend the $os_opts[$os]
value to it and declare it as a variable. The variable declaration is not necessary but my personal rule is; if you are going to use data more than once use a variable, if only once don't declare it.
You can do it like this. You can shorten your code like this.
$optionName='';//added some default values
$sendPush = false;;//added some default values
switch ($os) {
case "iOS":
$optionName='iOS_pastPushSavedID';
break;
case "Android":
$optionName='android_pastPushSavedID';
break;
case "Fire OS":
$optionName='fireos_pastPushSavedID';
break;
case "Safari":
$optionName='safari_pastPushSavedID';
break;
case "Chrome":
$optionName='chrome_pastPushSavedID';
break;
case "Firefox":
$optionName='firefox_pastPushSavedID';
break;
default:
$sendPush = false;
}
//this is operation which is common when $optionName is not empty.
if(!empty($optionName))
{
$optionData = get_option($optionName, $default = false);
if($post->ID != $optionData) {
update_option( $optionData, $post->ID, no);
$sendPush = true;
} else {
$sendPush = false;
}
}
Id write it more like this
function getOptionSpecifier() {
switch ($this->os) {
case "iOS":
return 'iOS_pastPushSavedID';
case "Android":
return 'android_pastPushSavedID';
case "Android":
return 'android_pastPushSavedID';
case "Fire OS":
return 'fireos_pastPushSavedID';
case "Safari":
return 'safari_pastPushSavedID';
case "Chrome":
return 'chrome_pastPushSavedID';
case "Firefox":
return 'firefox_pastPushSavedID';
default:
return '';
}
}
function send_notification($id) {
$optionSpecifier = getOptionSpecifier();
if ($optionSpecifier === NULL) {
return false;
}
$pastPushSavedID = get_option( $optionSpecifier, $default = false);
if($id != $pastPushSavedID) {
update_option( $optionSpecifier, $id, no);
return true;
//$title = ($os . '_New Push = ' . ' storedID: ' . $iOS_pastPushSavedID . ' / postID: ' . $post->ID);
} else {
//$title = ($os . '_Duplicate Push = ' . ' storedID: ' . $iOS_pastPushSavedID . ' / postID: ' . $post->ID);
return false;
}
}
$sendPush = send_notification($post->ID);
Multiple functions ala "separation of concerns" and so ...