City / Metrics / Helpers

[tomverran]

So currently the City class is relatively lightweight, containing only some basic methods to find tiles. The question is, should we add methods like countPopulation, countJobs etc to the city class or should they be a series of static functions that take a City as an argument, to keep the city class focused?

I initially went with option B, hence the Metrics class in World, but I don't know whether that is the best idea.

There's also helper methods like switchTypeOfRandom (or something) which I added to City, but I don't know whether I should also go with the different-class approach for that too.

So, ideas? :D Whatever happens we should be consistent, so I should either move the Metrics functions into the City class or the high level helper functions like swichTypeOfRandom out of the City class.

[Lukasa]

Personally, I'd move the metrics stuff into City. My understanding of OOP is that you should not be building classes that rely on implementation details of other classes, and the Metrics idea does. You should simply have a method on City called population() or getPopulation() or something, and it's the job of that method to understand how the City class works.

I feel like it's counter productive to insist on small classes. So long as the City class only provides stuff about the City, you haven't diluted its focus.

[tomverran]

Mm, your argument is what initially gave me pause for thought. I'll move the stuff into the city class I think.

[Lukasa]

Yeah, that stuff has actually been bogging me down. I wanted to create a single reference of Tile types (was initially going to be an enum), but the refactoring to do it is huge, because you have some repeated similar patterns (e.g. the call to determine population) which is a pain in the ass to keep changing. That stuff should probably be on the City class too.

[tomverran]

heh yeah, the tile types being strings is something I am uneasy about, I was going to put it into an issue actually. If you pull down the latest commit it slightly reduces the number of population calls by having getPopulation in the metrics class, so moving that to the city should fix most issues.

The thing is there's the basic tile stuff for getting by type etc, then the more gamey logic that says that commercial + industrial = jobs and I'm not sure whether we should be mixing that stuff into one class. Maybe the city class could be an abstract Map class or something, and its methods protected, then we have the City class with nice countJobs(), countPopulation() methods.

I think I might be overcomplicating things again :D

Oh also the reason I didn't make types an enum was because I was mixing up Java and PHP. I knew one of them didn't have enums but thought it was Java, silly me.

[Lukasa]

=P Easy to do, I had to look up the Java enum syntax to make sure I was declaring it properly.