TravelMapping / DataProcessing

Data Processing Scripts and Programs for Travel Mapping Project
4 stars 6 forks source link

dict indexing #167

Closed yakra closed 3 years ago

yakra commented 5 years ago

https://github.com/TravelMapping/DataProcessing/blob/5db8dfa9366dcc34bc7862041cd80fc3d86513b3/siteupdate/python-teresco/siteupdate.py#L2672-L2678 Does this comment still accurately describe what's going on here?

jteresco commented 5 years ago

I haven't touched anything there in a long time. Do you suspect things are now different than the comment describes?

yakra commented 5 years ago

# "clinched" dictionaries indexed by system name, values clinch count

This is as described. For con_routes_clinched at least; routes_clinched is commented out of course.

# "traveled" dictionaries indexed by system name, then conn or regular # route in another dictionary with keys route, values mileage

OK, this applies to con_routes_traveled. A dict of dicts, similar to system_region_mileages. I was thrown off a bit at first by the phrasing.

routes_traveled, OTOH, appears to be single-level dict, keys Route objects, values mileage.

One thing I like about C++ is, when I see

std::unordered_map<HighwaySystem*, std::unordered_map<Region*, double>> system_region_mileages;
std::unordered_map<HighwaySystem*, std::unordered_map<ConnectedRoute*, double>> con_routes_traveled;
std::unordered_map<Route*, double> routes_traveled;
std::unordered_map<HighwaySystem*, unsigned int> con_routes_clinched;

it's more immediately apparent what I'm dealing with ;)

jteresco commented 5 years ago

Please feel free to expand the comments to clarify.

You're definitely correct about the benefits of needing to declare types for data structures in languages like C++. It can be very difficult to keep track of what you've built in Python.

yakra commented 5 years ago

In terms of code organization, I think it makes sense to move the declaration of these three dicts into the TravelerList constructor, along with the three dicts that are already declared there.

yakra commented 3 years ago

These look like vestigial structures, currently unused (see #353).

yakra commented 3 years ago

Changes for #353 will remove these structures, and their comments along with them. :) Closing.