Skip to main content ↑ ↓
Topic: Code cleanup in dataobj/dingliste.cc (from Bernd Gabriel) (Read 2493 times) previous topic - next topic

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

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 ****essed in the depot constructors (I verified this), so it does not need to be done explicitly.

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

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

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

Reply #2 –
Quote from: prissi – on
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++ cl****es, 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.

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

Reply #3 –
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]
					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;

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

Reply #4 –
Quote from: prissi – on
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