[Freeciv-Dev] (PR#2566) PATCH: clean up handle_upgrade_unittype_request

["Guest via RT" <rt@xxxxxxxxxxxxxx>]

This patch cleans up the procedure handle_upgrade_unittype_request. Of 
course the behaviour is not changed. The improvements are: 
- All declared variables are now initialized in the declaration. 
- Constants are declared as such to prevent unintentional 
  modification. 
- There is no longer a return statement in the procedure. 
- The loop is more efficient because: 
  - "struct city * map_get_city(int, int)" is now only called when the 
    result will be used. 
  - The comparison of "pplayer->economic.gold" and "cost" is no longer 
    done in each iteration of the loop. It is only done when 
    "pplayer->economic.gold" has changed (and once before the loop is 
    entered). 
  - The variable "player_no" (declared as "const int") is used now 
    instead of "pplayer->player_no" to avoid access through a pointer 
    every time, which is unnecessary, because the value does not 
    change. 
  - The argument "from_unittype" (declared as "const int") is used 
    now instead of "packet->type" for the same reason as above. 
  - Use the simpler and thus more efficient prefix increment instead 
    of postfix increment. There was no reason for using postfix 
    increment here (there seldom is). 
- Some names have become more descriptive: 
    Old             New 
  - "packet->type"  "from_unittype" 
  - "to_unit"       "to_unittype" 
  - "upgraded"      "number_of_upgraded_units" 
    (the old name sounded like a name for a boolean). 
- The arguments have changed to better suit what the procedure needs. 
  This was trivial because the procedure is only called from 1 place. 
- Some styleguide violations have been eliminated: 
  - "to_unit =can_upgrade_unittype", which violated the rule about 
    spaces after operators. 
  - "if (cost > pplayer->economic.gold) 
       break;", which violated the rule about braces after 
    conditionals. 

diff -rXi -U2 freeciv/server/srv_main.c 
freeciv-cleanup_handle_upgrade_unittype_request/server/srv_main.c
--- freeciv/server/srv_main.c   2002-12-13 17:37:14.000000000 +0100
+++ freeciv-cleanup_handle_upgrade_unittype_request/server/srv_main.c   
2002-12-13 17:35:30.000000000 +0100
@@ -841,5 +841,5 @@
 
   case PACKET_UNITTYPE_UPGRADE:
-    handle_upgrade_unittype_request(pplayer, (struct packet_unittype_info 
*)packet);
+    handle_upgrade_unittype_request(pplayer, ((struct packet_unittype_info 
*)packet)->type);
     break;
 
diff -rXi -U2 freeciv/server/unithand.c 
freeciv-cleanup_handle_upgrade_unittype_request/server/unithand.c
--- freeciv/server/unithand.c   2002-12-13 17:37:14.000000000 +0100
+++ freeciv-cleanup_handle_upgrade_unittype_request/server/unithand.c   
2002-12-13 17:50:44.000000000 +0100
@@ -151,38 +151,43 @@
  Upgrade all units of a given type.
 **************************************************************************/
-void handle_upgrade_unittype_request(struct player *pplayer, 
-                                    struct packet_unittype_info *packet)
+void handle_upgrade_unittype_request
+(const struct player * const pplayer, const int from_unittype)
 {
-  int cost;
-  int to_unit;
-  int upgraded = 0;
-  if ((to_unit =can_upgrade_unittype(pplayer, packet->type)) == -1) {
-    notify_player(pplayer, _("Game: Illegal packet, can't upgrade %s (yet)."), 
-                 unit_types[packet->type].name);
-    return;
-  }
-  cost = unit_upgrade_price(pplayer, packet->type, to_unit);
-  conn_list_do_buffer(&pplayer->connections);
-  unit_list_iterate(pplayer->units, punit) {
-    struct city *pcity;
-    if (cost > pplayer->economic.gold)
-      break;
-    pcity = map_get_city(punit->x, punit->y);
-    if (punit->type == packet->type && pcity && pcity->owner == 
pplayer->player_no) {
-      pplayer->economic.gold -= cost;
-      
-      upgrade_unit(punit, to_unit);
-      upgraded++;
-    }
-  }
-  unit_list_iterate_end;
-  conn_list_do_unbuffer(&pplayer->connections);
-  if (upgraded > 0) {
-    notify_player(pplayer, _("Game: %d %s upgraded to %s for %d gold."), 
-                 upgraded, unit_types[packet->type].name, 
-                 unit_types[to_unit].name, cost * upgraded);
-    send_player_info(pplayer, pplayer);
+  const int to_unittype = can_upgrade_unittype(pplayer, from_unittype);
+  if (to_unittype == -1) {
+    notify_player(pplayer, _("Game: Illegal packet, can't upgrade %s (yet)."),
+                  unit_types[from_unittype].name);
   } else {
-    notify_player(pplayer, _("Game: No units could be upgraded."));
+    const int cost = unit_upgrade_price(pplayer, from_unittype, to_unittype);
+    int number_of_upgraded_units = 0;
+
+    if (pplayer->economic.gold >= cost) {
+      const int player_no = pplayer->player_no;
+      conn_list_do_buffer(&pplayer->connections);
+      unit_list_iterate(pplayer->units, punit) {
+        if (punit->type == from_unittype) {
+          const struct city * const pcity = map_get_city(punit->x, punit->y);
+          if (pcity && pcity->owner == player_no) {
+            upgrade_unit(punit, to_unittype);
+            ++number_of_upgraded_units;
+            if ((pplayer->economic.gold -= cost) < cost) {
+              break;
+            }
+          }
+        }
+      }
+      unit_list_iterate_end;
+      conn_list_do_unbuffer(&pplayer->connections);
+    }
+
+    if (number_of_upgraded_units > 0) {
+      notify_player(pplayer, _("Game: %d %s upgraded to %s for %d gold."),
+                    number_of_upgraded_units, unit_types[from_unittype].name,
+                    unit_types[to_unittype].name,
+                    cost * number_of_upgraded_units);
+      send_player_info(pplayer, pplayer);
+    } else {
+      notify_player(pplayer, _("Game: No units could be upgraded."));
+    }
   }
 }
diff -rXi -U2 freeciv/server/unithand.h 
freeciv-cleanup_handle_upgrade_unittype_request/server/unithand.h
--- freeciv/server/unithand.h   2002-12-13 17:37:14.000000000 +0100
+++ freeciv-cleanup_handle_upgrade_unittype_request/server/unithand.h   
2002-12-13 17:35:30.000000000 +0100
@@ -23,6 +23,6 @@
 void handle_unit_connect(struct player *pplayer, 
                          struct packet_unit_connect *req);
-void handle_upgrade_unittype_request(struct player *pplayer, 
-                                    struct packet_unittype_info *packet);
+void handle_upgrade_unittype_request
+(const struct player * const pplayer, const int from_unittype);
 void handle_unit_upgrade_request(struct player *pplayer,
                                 struct packet_unit_request *packet);