[patch] In rebuild_destinations()

Topic: [patch] In rebuild_destinations() (Read 2229 times) previous topic - next topic

[patch] In rebuild_destinations()

November 05, 2009, 11:01:08 am

This small patch (against r2848 relative to the trunk folder) fixes a small bug :

Quote

   // might be a new halt to add
   if( !warenziele_by_stops.is_contained(wzs) ) {
      warenziele_by_stops.append(wzs);
      non_identical_schedules_flag[ctg] = true;
   }

ctg should actually be add_catg_index[ctg].

It also changes the warenziele building loops so as to reduce the number of iterations required.

Re: [patch] In rebuild_destinations()

Reply #1 – November 05, 2009, 11:32:46 am

We must invent a new "Code diver" batch for you

Edit: Thanks for finding all these quirks and fixing them!

Re: [patch] In rebuild_destinations()

Reply #2 – November 05, 2009, 03:20:47 pm

Yeah, this code review is indeed extremely helpful. Thank you.

Re: [patch] In rebuild_destinations()

Reply #3 – November 05, 2009, 03:42:40 pm

It is just a small contribution, and it is my honour to be able to help

@ Prissi

Sorry for asking, but may I know why there are 2 loops that increment ctg??

Quote

for( uint8 ctg=0; ctg<add_catg_index.get_count(); ctg ++ ) {
   if(
      (add_catg_index[ctg]==0 && halt->get_pax_enabled()) ||
      (add_catg_index[ctg]==1 && halt->get_post_enabled()) ||
      (add_catg_index[ctg]>=2 && halt->get_ware_enabled())
   ) {
      do {
         warenzielsorter_t wzs( halt, stop_count, add_catg_index[ctg] );
         // might be a new halt to add
         if( !warenziele_by_stops.is_contained(wzs) ) {
            warenziele_by_stops.append(wzs);
            non_identical_schedules_flag[ctg] = true;
         }
      } while( add_catg_index[ctg++]>=2 && ctg<add_catg_index.get_count() );
   }
}

Does the inner do-while loop serve some special purpose?

Re: [patch] In rebuild_destinations()

Reply #4 – November 05, 2009, 04:16:18 pm

Because if goods are enabled, this means all goods are accepted there. p****=0, mail=1, all other goods>=2. THus the additional test above is not needed.

Re: [patch] In rebuild_destinations()

Reply #5 – November 05, 2009, 04:49:38 pm

However, the category indices in both add_catg_index (of lineless convoys) and goods_catg_index (of lines) are not sorted. Bulk goods may be processed before pax/mail, and if unfortunately pax/mail are not supported by the halt, warenziele will be wrong. Thus, there is no guarantee that the inner do-while loop works correctly under all circumstances.

Incidentally, you may just declare add_catg_index as a minivec instead of vector like a line's goods_catg_index.