[Freeciv-Dev] Re: Corrected code

[Raahul Kumar <raahul_da_man@xxxxxxxxx>]

 What about replacing the three lines above with:
   for (i = 0; i < (pcity->ppl_scientist -
 result->scientists); i++) {
 
 No extra variable. No extra if.
 
Not bad, but I like to create code that all fits on
one line. I find it harder to read code that splits
across
two lines, and I wanted to make the code self
documenting like you have. Nonetheless, you're
recommended changes 
are attached to this email. Does this finally wrap up
apply_result_on_server? 
        
 


__________________________________________________
Do You Yahoo!?
Get email alerts & NEW webcam video instant messaging with Yahoo! Messenger. 
http://im.yahoo.com

static void apply_result_on_server(struct city *pcity,
                                   const struct cma_result *const result)
{
  struct packet_city_request packet;
  int first_request_id = 0, last_request_id = 0, i,worker = 0;
struct cma_result current_state;

  get_current_as_result(pcity, &current_state);

  if (results_are_equal(result, &current_state)) {
    stats.apply_result_ignored++;
    return;
  }

  stats.apply_result_applied++;

  freelog(APPLY_RESULT_LOG_LEVEL, "apply_result(city='%s'(%d))",
          pcity->name, pcity->id);

  connection_do_buffer(&aconnection);

  packet.city_id = pcity->id;
  packet.name[0] = '\0';
  packet.worklist.name[0] = '\0';

  /*
   * Do checks
   */
  city_map_iterate(x, y) {
    if (x == 2 && y == 2)
      continue;

    if (result->worker_positions_used[x][y]) {
      worker++;
    }
  }
  city_map_iterate_end;

  if (pcity->size !=
      (worker + result->entertainers + result->scientists +
       result->taxmans)) {
    print_city(pcity);
    print_cma_result(result);
    assert(0);
  }

  /*
   * Remove all surplus workers: A surplus worker is defined as a worker not in 
the
   * correct (x,y) co-ordinates.
   */
  city_map_iterate(x, y) {
    if (x == 2 && y == 2)
      continue;
    if ((pcity->city_map[x][y] == C_TILE_WORKER) && 
(!result->worker_positions_used[x][y])) {
      last_request_id = set_worker(pcity, x, y, 0);
      if (!first_request_id) {
        first_request_id = last_request_id;
      }
    }
  }
  city_map_iterate_end;

  /*
   * Change surplus scientists to entertainers
   */
   
  for (i = 0; i < (pcity->ppl_scientist - result->scientists); i++) {
    packet.specialist_from = SP_SCIENTIST;
    packet.specialist_to = SP_ELVIS;
    last_request_id = send_packet_city_request(&aconnection, &packet,
                                               PACKET_CITY_CHANGE_SPECIALIST);
    if (!first_request_id) {
      first_request_id = last_request_id;
    }
  }

  /*
   * Change extra taxmans to entertainers
   */

  for (i = 0; i < (pcity->ppl_taxman - result->taxmans); i++) {
    packet.specialist_from = SP_TAXMAN;
    packet.specialist_to = SP_ELVIS;
    last_request_id = send_packet_city_request(&aconnection, &packet,
                                               PACKET_CITY_CHANGE_SPECIALIST);
    if (!first_request_id) {
      first_request_id = last_request_id;
    }
  }

  /* now all surplus people are enterainers */

  /*
   * Set workers
   */
  city_map_iterate(x, y) {
    if (x == 2 && y == 2)
      continue;

    if (result->worker_positions_used[x][y] && pcity->city_map[x][y] != 
C_TILE_WORKER) {
      last_request_id = set_worker(pcity, x, y, 1);
      if (!first_request_id) {
        first_request_id = last_request_id;
      }
    }
  }
  city_map_iterate_end;

  /*
   * Set scientists.
   */

  for (i = 0; i < (result->scientists - pcity->ppl_scientist); i++) {
    packet.specialist_from = SP_ELVIS;
    packet.specialist_to = SP_SCIENTIST;
    last_request_id = send_packet_city_request(&aconnection, &packet,
                                               PACKET_CITY_CHANGE_SPECIALIST);
    if (!first_request_id) {
      first_request_id = last_request_id;
    }
  }

  /*
   * Set taxmans.
   */
   
  for (i = 0; i < (result->taxmans - pcity->ppl_taxman); i++) {
    packet.specialist_from = SP_ELVIS;
    packet.specialist_to = SP_TAXMAN;
    last_request_id = send_packet_city_request(&aconnection, &packet,
                                               PACKET_CITY_CHANGE_SPECIALIST);
    if (!first_request_id) {
      first_request_id = last_request_id;
    }
  }

  connection_do_unbuffer(&aconnection);

  if (last_request_id) {
    wait_for_requests("CMA", first_request_id, last_request_id);
  }

  /*
   * Do some checks
   */
  get_current_as_result(pcity, &current_state);

  if (!results_are_equal(result, &current_state)) {
    freelog(LOG_NORMAL, "city='%s' expected result:", pcity->name);
    print_cma_result(result);
    freelog(LOG_NORMAL, "got:");
    print_cma_result(&current_state);
    assert(0);
  }

  freelog(APPLY_RESULT_LOG_LEVEL, "apply_result: return");
}