From 250e2dd768562fd1181d98dcaf68db23143bdaa7 Mon Sep 17 00:00:00 2001
From: Kevin Day <thekevinday@gmail.com>
Date: Sun, 2 Dec 2018 19:49:44 -0600
Subject: [PATCH] Cleanup: delay assignment of $this->value until end of
 do_build functions

---
 .../database/classes/database_alter_aggregate.php  | 19 ++++----
 .../database/classes/database_alter_coalation.php  | 20 ++++----
 .../database/classes/database_alter_conversion.php | 18 ++++----
 .../database/classes/database_alter_database.php   | 25 ++++++----
 .../classes/database_alter_default_privileges.php  | 54 +++++++++++-----------
 common/database/classes/database_alter_domain.php  |  1 +
 .../database/classes/database_alter_extension.php  |  2 +
 common/database/classes/database_query.php         | 12 +++--
 8 files changed, 84 insertions(+), 67 deletions(-)

diff --git a/common/database/classes/database_alter_aggregate.php b/common/database/classes/database_alter_aggregate.php
index eaabf32..a08feb1 100644
--- a/common/database/classes/database_alter_aggregate.php
+++ b/common/database/classes/database_alter_aggregate.php
@@ -301,10 +301,6 @@ class c_database_alter_aggregate extends c_database_query {
       return new c_base_return_false();
     }
 
-    // @fixme: use a local variable fo value before assigning.
-    $this->value = static::pr_QUERY_COMMAND;
-    $this->value .= ' ' . $this->name;
-
     $aggregate_signatures = NULL;
     if (!is_array($this->aggregate_modes) || empty($this->aggregate_modes)) {
       $aggregate_signatures = ' *';
@@ -353,23 +349,28 @@ class c_database_alter_aggregate extends c_database_query {
       unset($order_by_signatures);
     }
 
+    $value = NULL;
     if (is_string($this->rename_to)) {
-      $this->value .= ' ' . $aggregate_signatures . ' ' . $this->p_do_build_rename_to();
+      $action = $aggregate_signatures . ' ' . $this->p_do_build_rename_to();
     }
     else if (is_string($this->owner_to)) {
-      $this->value .= ' ' . $aggregate_signatures . ' ' . $this->p_do_build_owner_to();
+      $action = $aggregate_signatures . ' ' . $this->p_do_build_owner_to();
     }
     else if (is_string($this->set_schema)) {
-      $this->value .= ' ' . $aggregate_signatures . ' ' . $this->p_do_build_set_schema();
+      $action = $aggregate_signatures . ' ' . $this->p_do_build_set_schema();
     }
     else {
       unset($aggregate_signatures);
-
-      $this->value = NULL;
+      unset($action);
       return new c_base_return_false();
     }
     unset($aggregate_signatures);
 
+    $this->value = static::pr_QUERY_COMMAND;
+    $this->value .= ' ' . $this->name;
+    $this->value .= ' ' . $action;
+    unset($action);
+
     return new c_base_return_true();
   }
 }
diff --git a/common/database/classes/database_alter_coalation.php b/common/database/classes/database_alter_coalation.php
index d2fbca6..0058cb4 100644
--- a/common/database/classes/database_alter_coalation.php
+++ b/common/database/classes/database_alter_coalation.php
@@ -133,29 +133,31 @@ class c_database_alter_coalation extends c_database_query {
       return new c_base_return_false();
     }
 
-    // @fixme: use a local variable for value before returning.
-    $this->value = static::pr_QUERY_COMMAND;
-    $this->value .= ' ' . $this->name;
-
+    $action = NULL;
     if (is_bool($this->refresh_version)) {
       if ($this->refresh_version) {
-        $this->value .= ' ' . c_database_string::REFRESH_VERSION;
+        $action = c_database_string::REFRESH_VERSION;
       }
     }
     else if (is_string($this->rename_to)) {
-      $this->value .= ' ' . $this->p_do_build_rename_to();
+      $action = $this->p_do_build_rename_to();
     }
     else if (is_string($this->owner_to)) {
-      $this->value .= ' ' . $this->p_do_build_owner_to();
+      $action = $this->p_do_build_owner_to();
     }
     else if (is_string($this->set_schema)) {
-      $this->value .= ' ' . $this->p_do_build_set_schema();
+      $action = $this->p_do_build_set_schema();
     }
     else {
-      $this->value = NULL;
+      unset($action);
       return new c_base_return_false();
     }
 
+    $this->value = static::pr_QUERY_COMMAND;
+    $this->value .= ' ' . $this->name;
+    $this->value .= ' ' . $action;
+    unset($action);
+
     return new c_base_return_true();
   }
 }
diff --git a/common/database/classes/database_alter_conversion.php b/common/database/classes/database_alter_conversion.php
index e799c6c..28ee6e7 100644
--- a/common/database/classes/database_alter_conversion.php
+++ b/common/database/classes/database_alter_conversion.php
@@ -83,24 +83,26 @@ class c_database_alter_conversion extends c_database_query {
       return new c_base_return_false();
     }
 
-    // @fixme: use a local variable for value.
-    $this->value = static::pr_QUERY_COMMAND;
-    $this->value .= ' ' . $this->name;
-
+    $action = NULL;
     if (is_string($this->rename_to)) {
-      $this->value .= ' ' . $this->p_do_build_rename_to();
+      $action = $this->p_do_build_rename_to();
     }
     else if (is_string($this->owner_to)) {
-      $this->value .= ' ' . $this->p_do_build_owner_to();
+      $action = $this->p_do_build_owner_to();
     }
     else if (is_string($this->set_schema)) {
-      $this->value .= ' ' . $this->p_do_build_set_schema();
+      $action = $this->p_do_build_set_schema();
     }
     else {
-      $this->value = NULL;
+      unset($action);
       return new c_base_return_false();
     }
 
+    $this->value = static::pr_QUERY_COMMAND;
+    $this->value .= ' ' . $this->name;
+    $this->value .= ' ' . $action;
+    unset($action);
+
     return new c_base_return_true();
   }
 }
diff --git a/common/database/classes/database_alter_database.php b/common/database/classes/database_alter_database.php
index 66e6122..3afd766 100644
--- a/common/database/classes/database_alter_database.php
+++ b/common/database/classes/database_alter_database.php
@@ -147,29 +147,36 @@ class c_database_alter_database extends c_database_query {
       return new c_base_return_false();
     }
 
-    $this->value = static::pr_QUERY_COMMAND;
-    $this->value .= ' ' . $this->name;
-
+    $action = NULL;
     if ($this->option instanceof c_database_argument_database_option) {
       $this->option->do_build_argument();
-      $this->value .= ' ' . $this->option->get_value_exact();
+      $action = $this->option->get_value_exact();
     }
     else if (is_string($this->rename_to)) {
-      $this->value .= ' ' . $this->p_do_build_rename_to();
+      $action = $this->p_do_build_rename_to();
     }
     else if (is_string($this->owner_to)) {
-      $this->value .= ' ' . $this->p_do_build_owner_to();
+      $action = $this->p_do_build_owner_to();
     }
     else if (is_string($this->set_tablespace)) {
-      $this->value .= ' ' . $this->p_do_build_set_tablespace();
+      $action = $this->p_do_build_set_tablespace();
     }
     else if (is_array($this->set)) {
-        $this->value .= ' ' . $this->p_do_build_set();
+      $action = $this->p_do_build_set();
     }
     else if (is_array($this->reset)) {
-      $this->value .= ' ' . $this->p_do_build_reset();
+      $action = $this->p_do_build_reset();
+    }
+    else {
+      unset($action);
+      return new c_base_return_false();
     }
 
+    $this->value = static::pr_QUERY_COMMAND;
+    $this->value .= ' ' . $this->name;
+    $this->value .= ' ' . $action;
+    unset($action);
+
     return new c_base_return_true();
   }
 }
diff --git a/common/database/classes/database_alter_default_privileges.php b/common/database/classes/database_alter_default_privileges.php
index a55af00..ac6c43f 100644
--- a/common/database/classes/database_alter_default_privileges.php
+++ b/common/database/classes/database_alter_default_privileges.php
@@ -439,35 +439,29 @@ class c_database_alter_default_priveleges extends c_database_query {
           return new c_base_return_false();
     }
 
-    // @fixme: use a local variable and only assign value once at the end after any potential error cases.
-    $this->value = static::pr_QUERY_COMMAND;
-
     // [ FOR ROLE target_role [, ... ] ]
+    $action = NULL;
     if (is_array($this->for_role_targets) && !empty($this->for_role_targets)) {
-      $this->value .= ' ' . c_database_string::FOR . ' ' . c_database_string::ROLE;
-
-      $names = NULL;
-      foreach ($this->for_role_targets as $schema_name) {
-        $names .= ', ' . $schema_name;
-      }
-
-      $this->value .= ltrim($names, ',');
-      unset($names);
+      $action = c_database_string::FOR . ' ' . c_database_string::ROLE;
+      $action .= ' ' . implode(', ', $this->for_role_targets);
     }
 
     // [ IN SCHEMA schema_name [, ... ] ]
     if (is_array($this->in_schema) && !empty($this->in_schema)) {
-      $this->value .= $this->p_do_build_in_schema();
+      $action .= is_null($action) ? '' : ' ';
+      $action .= $this->p_do_build_in_schema();
     }
 
     if ($this->action === e_database_action::ACTION_GRANT) {
-      $this->value .= ' ' . c_database_string::GRANT;
+      $action .= is_null($action) ? '' : ' ';
+      $action .= c_database_string::GRANT;
     }
     else if ($this->action === e_database_action::ACTION_REVOKE) {
-      $this->value .= ' ' . c_database_string::REVOKE;
+      $action .= is_null($action) ? '' : ' ';
+      $action .= c_database_string::REVOKE;
 
       if ($this->option_grant) {
-        $this->value .= ' ' . c_database_string::GRANT_OPTION_FOR;
+        $action .= ' ' . c_database_string::GRANT_OPTION_FOR;
       }
     }
 
@@ -514,34 +508,35 @@ class c_database_alter_default_priveleges extends c_database_query {
       }
     }
 
-    $this->value .= ltrim($privileges, ',');
+    $action .= is_null($action) ? '' : ' ';
+    $action .= ltrim($privileges, ', ');
     unset($privileges);
 
     // ON ...
     switch($this->on) {
       case e_database_on::TABLES_TO:
-        $this->value .= ' ' . c_database_string::ON_TABLES_TO;
+        $action .= ' ' . c_database_string::ON_TABLES_TO;
         break;
       case e_database_on::SEQUENCES:
-        $this->value .= ' ' . c_database_string::ON_SEQUENCES;
+        $action .= ' ' . c_database_string::ON_SEQUENCES;
         break;
       case e_database_on::FUNCTIONS:
-        $this->value .= ' ' . c_database_string::ON_FUNCTIONS;
+        $action .= ' ' . c_database_string::ON_FUNCTIONS;
         break;
       case e_database_on::TYPES:
-        $this->value .= ' ' . c_database_string::ON_TYPES;
+        $action .= ' ' . c_database_string::ON_TYPES;
         break;
       case e_database_on::SCHEMAS:
-        $this->value .= ' ' . c_database_string::ON_SCHEMAS;
+        $action .= ' ' . c_database_string::ON_SCHEMAS;
         break;
     }
 
     // [ TO | FROM ] ... role names ...
     if ($this->action === e_database_action::GRANT) {
-      $this->value .= ' ' . c_database_string::TO;
+      $action .= ' ' . c_database_string::TO;
     }
     else if ($this->action === e_database_action::REVOKE) {
-      $this->value .= ' ' . c_database_string::FROM;
+      $action .= ' ' . c_database_string::FROM;
     }
 
     foreach ($this->role_names as $role_name) {
@@ -550,25 +545,30 @@ class c_database_alter_default_priveleges extends c_database_query {
       }
 
       $role_name->do_build_argument();
-      $this->value .= ' ' . $role_name->get_value_exact();
+      $action .= ' ' . $role_name->get_value_exact();
     }
     unset($role_name);
 
     if ($this->action === e_database_action::GRANT) {
       // [ WITH GRANT OPTION ]
       if ($this->option_grant) {
-        $this->value .= ' ' . c_database_string::WITH_GRANT_OPTION;
+        $action .= ' ' . c_database_string::WITH_GRANT_OPTION;
       }
     }
     else if ($this->action === e_database_action::REVOKE) {
       // [ CASCADE | RESTRICT ]
       $option = $this->p_do_build_option();
       if (is_string($option)) {
-        $this->value .= ' ' . $option;
+        $action .= ' ' . $option;
       }
       unset($option);
     }
 
+    $this->value = static::pr_QUERY_COMMAND;
+    $this->value .= ' ' . $this->name;
+    $this->value .= ' ' . $action;
+    unset($action);
+
     return new c_base_return_true();
   }
 }
diff --git a/common/database/classes/database_alter_domain.php b/common/database/classes/database_alter_domain.php
index e5b64e9..0e7290d 100644
--- a/common/database/classes/database_alter_domain.php
+++ b/common/database/classes/database_alter_domain.php
@@ -347,6 +347,7 @@ class c_database_alter_coalation extends c_database_query {
     $this->value = static::pr_QUERY_COMMAND;
     $this->value .= ' ' . $this->name;
     $this->value .= ' ' . $action;
+    unset($action);
 
     return new c_base_return_true();
   }
diff --git a/common/database/classes/database_alter_extension.php b/common/database/classes/database_alter_extension.php
index 017a7b8..87fc0e5 100644
--- a/common/database/classes/database_alter_extension.php
+++ b/common/database/classes/database_alter_extension.php
@@ -136,7 +136,9 @@ class c_database_alter_extension extends c_database_query {
         unset($action);
         return new c_base_return_false();
     }
+    unset($action);
 
+    $this->value = $action;
     return new c_base_return_true();
   }
 }
diff --git a/common/database/classes/database_query.php b/common/database/classes/database_query.php
index d669744..af2941f 100644
--- a/common/database/classes/database_query.php
+++ b/common/database/classes/database_query.php
@@ -1449,26 +1449,28 @@ class c_database_argument_role_name extends c_base_return_string implements i_da
       return new c_base_return_false();
     }
 
-    $this->value = '';
+    $value = '';
     if ($this->argument_mode === static::QUERY_ARGUMENT_MODE_PUBLIC) {
-      $this->value .= ' ' . c_database_string::PUBLIC;
+      $value .= ' ' . c_database_string::PUBLIC;
     }
     else if ($this->argument_mode === static::QUERY_ARGUMENT_MODE_NAME) {
       if (is_string($this->argument_name)) {
-        $this->value .= ' ' . $this->argument_name;
+        $value .= ' ' . $this->argument_name;
       }
     }
     else if ($this->argument_mode === static::QUERY_ARGUMENT_MODE_GROUP) {
-      $this->value .= ' ' . c_database_string::GROUP;
+      $value .= ' ' . c_database_string::GROUP;
 
       if (is_string($this->argument_name)) {
-        $this->value .= ' ' . $this->argument_name;
+        $value .= ' ' . $this->argument_name;
       }
     }
     else {
+      unset($value);
       return new c_base_return_false();
     }
 
+    $this->value = $value;
     return new c_base_return_true();
   }
 
-- 
1.8.3.1