Code cleanup in dataobj/dingliste.cc (from Bernd Gabriel)

[neroden]

More in the project of eliminating silly differences between standard and experimental, Bernd cleaned up the code here for experimental, no behavior change, so this should be good for standard.  I took the liberty of removing the commented-out code he left behind.  Note that the maintenance costs are assessed in the depot constructors (I verified this), so it does not need to be done explicitly.

[prissi]

This will fail with crash, if the depot in question is not available (for instance a new version left it out ... )

[neroden]

This will fail with crash, if the depot in question is not available (for instance a new version left it out ... )

And the previous code won't?  I'm not seeing the change.

The changes are:
-- replacement of an if-else if chain with a 'case' statement -- should not cause crashes
-- inclusion of gb->get_besitzer() in the constructor calls for new monorail_depot etc. instead of setting it later -- should not cause crashes
-- not running add_maintenance() twice -- cannot possibly cause crashes
-- Using the fact that tramdepot_t and monoraildepot_t inherit, as C++ classes, from bahndepot_t -- should not cause crashes

So I don't know what you're talking about, but I'm pretty sure that the code cleanup does not change whatever it is.

[prissi]

Ok, I found my error. First cause crashes whille rdwr_vehicles, other with set_besitzer ... But why the switch statement? This is rather longer than a cleanup of the original like below.

Code Select Expand


gebaeude_t *gb = new gebaeude_t(welt, file);
if(gb->get_tile()->get_besch()->get_extra()==monorail_wt) {
d = new monoraildepot_t(welt, gb->get_pos(), gb->get_besitzer(), gb->get_tile());
}
else if(gb->get_tile()->get_besch()->get_extra()==tram_wt) {
d = new tramdepot_t(welt, gb->get_pos(), gb->get_besitzer(), gb->get_tile());
}
else {
d = new bahndepot_t(welt, gb->get_pos(), gb->get_besitzer(), gb->get_tile());
}
((bahndepot_t *)d)->rdwr_vehicles( file );
typ = d->get_typ();
// do not remove from this position, since there will be nothing
gb->set_flag(ding_t::not_on_map);
delete gb;


[neroden]

Ok, I found my error. First cause crashes whille rdwr_vehicles, other with set_besitzer ... But why the switch statement? This is rather longer than a cleanup of the original like below.

Your if-else-if version looks just fine too; if you commit that version I'll merge it back to experimental.  I expect they should both compile exactly the same on an optimizing compiler.

EDIT never mind, I see an even better version has gone in already