Address review comments, and fix other bugs

Remove some unused "$key" variables in foreach statements. Use ?? operator instead of checking isset() in some places. Signed-off-by: Nathaniel Clark <[email protected]>
utopiabound · Oct 20, 2021 · 603aa7d · 603aa7d
1 parent 3c9bcb7
commit 603aa7d
Show file tree

Hide file tree

Showing 14 changed files with 36 additions and 41 deletions.
diff --git a/admin/s_language/index.php b/admin/s_language/index.php
@@ -377,8 +377,9 @@ function update_lang_vars($HTTP_VARS) {
 	} else {
 		echo ("<div class=\"error\">" . get_opendb_lang_var('operation_not_available') . "</div>");
 	}
+
 } else if ($HTTP_VARS['op'] == '') {
-	if (is_not_empty_array($errors))
+	if (is_not_empty_array($errors ?? NULL))
 		echo format_error_block($errors);
 
 	// list languages and options

diff --git a/item_display.php b/item_display.php
@@ -101,9 +101,9 @@
 							}
 							echo ("<img src=\"" . $file_r['thumbnail']['url'] . "\" title=\"" . htmlspecialchars($coverimage_r['prompt']) . "\" ");
 
-							if (is_numeric($file_r['thumbnail']['width'] ?? ""))
+							if (is_numeric($file_r['thumbnail']['width'] ?? FALSE))
 								echo (' width="' . $file_r['thumbnail']['width'] . '"');
-							if (is_numeric($file_r['thumbnail']['height'] ?? ""))
+							if (is_numeric($file_r['thumbnail']['height'] ?? FALSE))
 								echo (' height="' . $file_r['thumbnail']['height'] . '"');
 
 							echo (">");

diff --git a/lib/TitleMask.class.php b/lib/TitleMask.class.php
@@ -842,7 +842,7 @@ function _get_title_mask_idx($s_item_type_group, $s_item_type) {
 			$item_type_group_r = fetch_item_type_groups_for_item_type_r ( $s_item_type, 'Y' );
 			if (is_array ( $item_type_group_r )) {
 				reset ( $item_type_group_r );
-				foreach ( $item_type_group_r as $key => $group ) {
+				foreach ( $item_type_group_r as $group ) {
 					$index = $this->_find_title_mask_idx ( $group, NULL );
 					if ($index != - 1) {
 						// cache mapping

diff --git a/lib/admin.php b/lib/admin.php
@@ -172,6 +172,7 @@ function execute_sql_install($ADMIN_TYPE, $sqlfile, &$errors) {
 function generate_sql_list($ADMIN_TYPE, $typeName, $sqlRegexp, $is_not_exists_function) {
 	$filelist = get_file_list ( './admin/' . $ADMIN_TYPE . '/sql/', 'sql' );
 	$sitelist = NULL;
+	$sqllist = NULL;
 	if (is_not_empty_array ( $filelist )) {
 		for($i = 0; $i < count ( $filelist ); $i ++) {
 			$parsedfile_r = parse_file ( $filelist [$i] );

diff --git a/lib/config.php b/lib/config.php
@@ -244,8 +244,8 @@ function get_db_config_var($type, $vars_r, $group, $id, $keyid) {
 	if (count ( $vars_r ) > 1) {
 		if ($type == 'boolean') {
 			$boolean_vars_r = NULL;
-			reset ( $vars_r );
-			foreach ($vars_r as $key=>$value) {
+			reset( $vars_r );
+			foreach ($vars_r as $key => $value) {
 				if ($value == 'TRUE')
 					$boolean_vars_r[$key] = TRUE;
 				else //if($value == 'FALSE')

diff --git a/lib/filecache.php b/lib/filecache.php
@@ -249,12 +249,10 @@ function file_cache_get_image_r($url, $type) {
 			}
 		}
 
-		if (! is_numeric ( $file_r['thumbnail']['width'] ?? '' ) && ! is_numeric ( $file_r['thumbnail']['height'] ?? '' )) {
-			if (isset($thumbnail_size_r ['width']))
-				$file_r ['thumbnail'] ['width'] = $thumbnail_size_r ['width'];
-			if (isset($thumbnail_size_r ['height']))
-				$file_r ['thumbnail'] ['height'] = $thumbnail_size_r ['height'];
-		}
+		if (is_numeric($thumbnail_size_r ['width'] ?? ''))
+			$file_r ['thumbnail'] ['width'] = $thumbnail_size_r ['width'];
+		if (is_numeric($thumbnail_size_r ['height'] ?? ''))
+			$file_r ['thumbnail'] ['height'] = $thumbnail_size_r ['height'];
 
 		return $file_r;
 	}

diff --git a/lib/inputfields.php b/lib/inputfields.php
@@ -68,7 +68,7 @@ function get_item_input_field($fieldname, $item_attribute_type_r, $item_r, $valu
 		$widget ['args'] [4] = $item_attribute_type_r ['input_type_arg5'];
 	}
 
-	if (isset($item_attribute_type_r ['multi_attribute_ind']) && $item_attribute_type_r ['multi_attribute_ind'] == 'Y') {
+	if (($item_attribute_type_r['multi_attribute_ind'] ?? '') == 'Y') {
 		$multi_value = TRUE;
 
 		if (! is_array ( $value )) {
@@ -717,7 +717,7 @@ function url($name, $item_r, $item_attribute_type_r, $prompt, $length, $maxlengt
 		$onchange = "onchange=\"$onchange_event\"";
 	}
 
-	if (isset($item_attribute_type_r ['file_attribute_ind']) && $item_attribute_type_r ['file_attribute_ind'] == 'Y') {
+	if (($item_attribute_type_r ['file_attribute_ind'] ?? '') == 'Y') {
 		$field .= "\n<ul class=\"urlOptionsMenu\" id=\"${name}-tab-menu\" class=\"file-upload-menu\">";
 		$field .= "<li id=\"menu-${name}_saveurl\" class=\"activeTab\" onclick=\"return activateTab('${name}_saveurl', '${name}-tab-menu', '${name}-tab-content', 'activeTab', 'fieldContent');\">URL</li>";
 		if (is_file_upload_enabled ()) {

diff --git a/lib/item.php b/lib/item.php
@@ -830,9 +830,9 @@ function from_and_where_clause($HTTP_VARS, $column_display_config_rs = NULL, $qu
 	//
 	// Owner restriction
 	//
-	if (isset( $HTTP_VARS ['owner_id'] ))
+	if (strlen($HTTP_VARS ['owner_id'] ?? '') > 0)
 		$where_r [] = 'ii.owner_id = \'' . $HTTP_VARS ['owner_id'] . '\'';
-	else if (isset( $HTTP_VARS ['not_owner_id'] )) //For not showing current user items.
+	else if (strlen( $HTTP_VARS ['not_owner_id'] ?? '' ) > 0) //For not showing current user items.
 		$where_r [] = 'ii.owner_id <> \'' . $HTTP_VARS ['not_owner_id'] . '\'';
 
 	//
@@ -843,12 +843,11 @@ function from_and_where_clause($HTTP_VARS, $column_display_config_rs = NULL, $qu
 		strlen( $HTTP_VARS['s_item_type'] ) > 0)
 	{
 		$where_r [] = 'i.s_item_type = \'' . $HTTP_VARS ['s_item_type'] . '\'';
-	} else if (isset( $HTTP_VARS ['s_item_type_group'] )) {
+	} else if (strlen( $HTTP_VARS ['s_item_type_group'] ?? '') > 0) {
 		$from_r [] = 's_item_type_group_rltshp sitgr';
 		$where_r [] = 'sitgr.s_item_type = i.s_item_type';
 		$where_r [] = 'sitgr.s_item_type_group = \'' . $HTTP_VARS ['s_item_type_group'] . '\'';
-	} else if (isset( $HTTP_VARS['s_item_type'] ) &&
-			   is_not_empty_array( $HTTP_VARS['s_item_type'] )) {
+	} else if (is_not_empty_array( $HTTP_VARS['s_item_type'] ?? '' )) {
 		$where_r [] = 'i.s_item_type IN(' . format_sql_in_clause ( $HTTP_VARS ['s_item_type'] ) . ')';
 	}
 
@@ -858,11 +857,9 @@ function from_and_where_clause($HTTP_VARS, $column_display_config_rs = NULL, $qu
 	//
 	// Status Type restriction
 	//
-	if (isset( $HTTP_VARS['s_status_type'] ) &&
-		is_not_empty_array ( $HTTP_VARS['s_status_type'] )) {
+	if (is_not_empty_array( $HTTP_VARS['s_status_type'] ?? '' )) {
 		$where_r [] = 'sst.s_status_type IN(' . format_sql_in_clause ( $HTTP_VARS ['s_status_type'] ) . ')';
-	} else if (isset( $HTTP_VARS ['s_status_type'] ) &&
-			   $HTTP_VARS ['s_status_type'] != 'ALL' ) {
+	} else if (($HTTP_VARS ['s_status_type'] ?? '') != 'ALL' ) {
 		$where_r [] = 'sst.s_status_type = \'' . $HTTP_VARS ['s_status_type'] . '\'';
 	}
 
@@ -874,8 +871,7 @@ function from_and_where_clause($HTTP_VARS, $column_display_config_rs = NULL, $qu
 	//
 	// User and Status type restriction
 	//
-	if (isset($HTTP_VARS['owner_id']) &&
-		strcmp( $HTTP_VARS['owner_id'], get_opendb_session_var( 'user_id' ) ) !== 0) {	// not current user
+	if (strcmp( ($HTTP_VARS['owner_id'] ?? ''), get_opendb_session_var( 'user_id' ) ) !== 0) {	// not current user
 		$from_r [] = 'user u';
 		$where_r [] = 'u.user_id = ii.owner_id';
 		$where_r [] = 'u.active_ind = \'Y\'';
@@ -884,7 +880,7 @@ function from_and_where_clause($HTTP_VARS, $column_display_config_rs = NULL, $qu
 	//
 	// Status Comment restriction
 	//
-	if (isset( $HTTP_VARS['status_comment'] )) {
+	if (strlen( $HTTP_VARS['status_comment'] ?? '') > 0) {
 		// Escape only the single quote!
 		$HTTP_VARS['status_comment'] = str_replace( "'", "\\'", $HTTP_VARS['status_comment'] );
 
@@ -908,7 +904,7 @@ function from_and_where_clause($HTTP_VARS, $column_display_config_rs = NULL, $qu
 	//
 	// Title restriction
 	//
-	if (isset( $HTTP_VARS ['title'] )) {
+	if (strlen( $HTTP_VARS ['title'] ?? '') > 0) {
 		// Escape only the single quote!
 		$HTTP_VARS ['title'] = str_replace ( "'", "\\'", $HTTP_VARS ['title'] );
 

diff --git a/lib/language.php b/lib/language.php
@@ -150,7 +150,7 @@ function get_opendb_lang_var($varname, $find = NULL, $replace = NULL) {
 	global $LANG_VARS;
 	global $_OPENDB_LANGUAGE;
 
-	if (!isset($LANG_VARS[$varname]) || strlen( $LANG_VARS[$varname] ) == 0) {
+	if (strlen( $LANG_VARS[$varname] ?? '' ) == 0) {
 		$value = fetch_opendb_db_lang_var( $_OPENDB_LANGUAGE, $varname );
 		if ($value !== FALSE) {
 			$LANG_VARS[$varname] = $value;
@@ -277,7 +277,7 @@ function get_system_table_r() {
 	$tables_r = NULL;
 
 	reset ( $_OPENDB_LANG_SYSTEM_TABLES );
-	foreach ( $_OPENDB_LANG_SYSTEM_TABLES as $key => $table ) {
+	foreach ( $_OPENDB_LANG_SYSTEM_TABLES as $table => $_v ) {
 		$tables_r [] = $table;
 	}
 

diff --git a/lib/rss.php b/lib/rss.php
@@ -48,7 +48,7 @@ function get_opendb_rss_feed_config($feed) {
 	reset ( $feeds_r );
 
 	$buffer = '';
-	foreach ($feeds_r as $key => $feed_r) {
+	foreach ($feeds_r as $feed_r) {
 		if ($feed_r ['feed'] == $feed) {
 			return $feed_r;
 		}
@@ -63,7 +63,7 @@ function get_opendb_rss_feeds_links($browser = 'firefox') {
 
 	$buffer = "\n";
 
-	foreach ($feeds_r as $key => $feed_r) {
+	foreach ($feeds_r as $feed_r) {
 		$buffer .= "<link rel=\"alternate\" type=\"application/rss+xml\" title=\"" . $feed_r ['title'] . "\" href=\"./rss.php?feed=" . $feed_r ['feed'] . "\">\n";
 	}
 	return $buffer;

diff --git a/lib/utils.php b/lib/utils.php
@@ -89,7 +89,7 @@ function generate_random_num() {
 	instead.
 */
 function ifempty($value, $ifnull) {
-	if (isset($value) && strlen( $value ) > 0)
+	if (strlen( $value ?? '' ) > 0)
 		return $value;
 	else
 		return $ifnull;
@@ -101,7 +101,7 @@ function ifempty($value, $ifnull) {
 	combination which is pissing me off.
 */
 function is_not_empty_array($array) {
-	if (isset( $array ) && is_array( $array ) && count( $array ) > 0)
+	if (is_array( $array ?? '' ) && count( $array ) > 0)
 		return TRUE;
 	else
 		return FALSE;

diff --git a/lib/welcome/WhatsNew.class.php b/lib/welcome/WhatsNew.class.php
@@ -31,13 +31,13 @@ function renderBlock($userid, $lastvisit) {
 		$whats_new_rs = get_whats_new_details($lastvisit, $userid);
 		if (is_array($whats_new_rs)) {
 			$buffer = "\n<ul>";
-			foreach ( $whats_new_rs as $key => $whats_new_r ) {
+			foreach ( $whats_new_rs as $whats_new_r ) {
 				if (is_array($whats_new_r['items'])) {
 					$buffer .= "\n<li><ul class=\"block\">";
 					$buffer .= "\n<h4>" . $whats_new_r['heading'] . "</h4>";
 
 					reset($whats_new_r['items']);
-					foreach ( $whats_new_r['items'] as $k => $item_rs ) {
+					foreach ( $whats_new_r['items'] as $item_rs ) {
 						$buffer .= "\n<li class=\"" . $item_rs['class'] . "\">" . $item_rs['content'] . "</li>";
 					}
 					$buffer .= "\n</ul></li>";

diff --git a/lib/widgets.php b/lib/widgets.php
@@ -98,7 +98,8 @@ function custom_select($name, $lookup_results, $display_mask, $size = 1, $value
 	$empty_display_mask = expand_display_mask ( $display_mask, NULL, '%' );
 
 	$value_found = FALSE;
-	foreach ( $lookup_results as $lookup_r ) {
+	foreach ( $lookup_results as $lookup_r_key => $lookup_r_value ) {
+		$lookup_r = ["key" => $lookup_r_key, "value" => $lookup_r_value];
 		// Check if this record should be included in list of values.
 		if (! function_exists ( $include_ind_func ) || $include_ind_func ( $lookup_r )) {
 			$lookup_value = get_array_variable_value ( $lookup_r, $value_column );

diff --git a/listings.php b/listings.php
@@ -131,10 +131,8 @@ function getListingFiltersBlock() {
 function fetch_cached_attribute_type_r($s_attribute_type) {
 	global $_OPENDB_DB_CACHE;
 
-	if (!isset($_OPENDB_DB_CACHE['s_attribute_type']) ||
-        !is_array($_OPENDB_DB_CACHE['s_attribute_type']) ||
-		!isset($_OPENDB_DB_CACHE['s_attribute_type'][$s_attribute_type]) ||
-        !is_array($_OPENDB_DB_CACHE['s_attribute_type'][$s_attribute_type])) {
+	if (!is_array($_OPENDB_DB_CACHE['s_attribute_type'] ?? '') ||
+		!is_array($_OPENDB_DB_CACHE['s_attribute_type'][$s_attribute_type] ?? '')) {
 		$_OPENDB_DB_CACHE['s_attribute_type'][$s_attribute_type] = fetch_attribute_type_r($s_attribute_type);
 	}
 
@@ -309,7 +307,7 @@ function get_column_display_config(&$HTTP_VARS, $show_owner_column, $show_action
 		$v_column_display_config_rs = &filter_for_printable_list($v_column_display_config_rs);
 	}
 
-	if (isset($HTTP_VARS['attr_match']) && $HTTP_VARS['attr_match'] != 'category' && strlen($HTTP_VARS['attribute_type']) > 0) {
+	if (($HTTP_VARS['attr_match'] ?? '') != 'category' && strlen($HTTP_VARS['attribute_type']) > 0) {
 		// Now we have to merge in search terms, and add them after the 'title' column_id
 		$v_column_display_config_rs = &merge_display_column_config_arrays(
 			$v_column_display_config_rs,