From 8bcc7644c5dd32821ce38fa042ef69c6d8700266 Mon Sep 17 00:00:00 2001 From: eric bryant Date: Mon, 4 May 2020 01:10:13 -0400 Subject: [PATCH 1/2] refactor roots & list names; smash case * Refactor checks for root & list name uniqueness and Route/ConnectedRoute correspondence for improved efficiency & error reporting and more concise code (#316) * Perform fewer case conversions on AltLabels & AltRouteNames for increased speed Processing traveler list files (#319) * C++: clean up a couple classes' static member definitions --- .../classes/ConnectedRoute/ConnectedRoute.cpp | 23 ++- .../classes/ConnectedRoute/ConnectedRoute.h | 2 +- .../classes/GraphGeneration/HGEdge.cpp | 5 - .../classes/GraphGeneration/HGEdge.h | 6 +- .../classes/GraphGeneration/PlaceRadius.cpp | 16 +- .../cplusplus/classes/HighwaySystem.cpp | 8 +- siteupdate/cplusplus/classes/Route/Route.cpp | 31 ++- siteupdate/cplusplus/classes/Route/Route.h | 1 + .../cplusplus/classes/Route/read_wpt.cpp | 10 - .../classes/TravelerList/TravelerList.cpp | 34 ++-- .../cplusplus/classes/Waypoint/Waypoint.cpp | 19 +- .../cplusplus/classes/Waypoint/Waypoint.h | 1 - siteupdate/cplusplus/functions/lower.cpp | 4 +- siteupdate/cplusplus/functions/upper.cpp | 6 + siteupdate/cplusplus/siteupdate.cpp | 82 ++------ .../cplusplus/threads/ReadListThread.cpp | 4 +- siteupdate/python-teresco/siteupdate.py | 189 +++++++----------- 17 files changed, 179 insertions(+), 262 deletions(-) diff --git a/siteupdate/cplusplus/classes/ConnectedRoute/ConnectedRoute.cpp b/siteupdate/cplusplus/classes/ConnectedRoute/ConnectedRoute.cpp index 5e0ddd85..c043df6b 100644 --- a/siteupdate/cplusplus/classes/ConnectedRoute/ConnectedRoute.cpp +++ b/siteupdate/cplusplus/classes/ConnectedRoute/ConnectedRoute.cpp @@ -1,4 +1,4 @@ -ConnectedRoute::ConnectedRoute(std::string &line, HighwaySystem *sys, ErrorList &el, std::list &route_list) +ConnectedRoute::ConnectedRoute(std::string &line, HighwaySystem *sys, ErrorList &el) { mileage = 0; // parse chopped routes csv line @@ -29,19 +29,28 @@ ConnectedRoute::ConnectedRoute(std::string &line, HighwaySystem *sys, ErrorList el.add_error("groupname > " + std::to_string(DBFieldLength::city) + " bytes in " + system->systemname + "_con.csv line: " + line); // roots + lower(roots_str.data()); int rootOrder = 0; size_t l = 0; for (size_t r = 0; r != -1; l = r+1) { r = roots_str.find(',', l); - Route *root = route_by_root(roots_str.substr(l, r-l), route_list); - if (!root) el.add_error("Could not find Route matching ConnectedRoute root " + roots_str.substr(l, r-l) + - " in system " + system->systemname + '.'); - else { roots.push_back(root); + try { Route *root = Route::root_hash.at(roots_str.substr(l, r-l)); + roots.push_back(root); + if (root->con_route) + el.add_error("Duplicate root in " + sys->systemname + "_con.csv: " + root->root + + " already in " + root->con_route->system->systemname + "_con.csv"); + if (system != root->system) + el.add_error("System mismatch: chopped route " + root->root + " from " + root->system->systemname + + ".csv in connected route in " + system->systemname + "_con.csv"); root->con_route = this; // save order of route in connected route root->rootOrder = rootOrder; - } - rootOrder++; + rootOrder++; + } + catch (std::out_of_range& oor) + { el.add_error("Could not find Route matching ConnectedRoute root " + roots_str.substr(l, r-l) + + " in system " + system->systemname + '.'); + } } if (roots.size() < 1) el.add_error("No roots in " + system->systemname + "_con.csv line: " + line); } diff --git a/siteupdate/cplusplus/classes/ConnectedRoute/ConnectedRoute.h b/siteupdate/cplusplus/classes/ConnectedRoute/ConnectedRoute.h index c9d05cfe..9692f937 100644 --- a/siteupdate/cplusplus/classes/ConnectedRoute/ConnectedRoute.h +++ b/siteupdate/cplusplus/classes/ConnectedRoute/ConnectedRoute.h @@ -12,7 +12,7 @@ class ConnectedRoute double mileage; // will be computed for routes in active & preview systems - ConnectedRoute(std::string &, HighwaySystem *, ErrorList &, std::list &); + ConnectedRoute(std::string &, HighwaySystem *, ErrorList &); std::string connected_rtes_line(); std::string csv_line(); diff --git a/siteupdate/cplusplus/classes/GraphGeneration/HGEdge.cpp b/siteupdate/cplusplus/classes/GraphGeneration/HGEdge.cpp index 579d8e81..24158034 100644 --- a/siteupdate/cplusplus/classes/GraphGeneration/HGEdge.cpp +++ b/siteupdate/cplusplus/classes/GraphGeneration/HGEdge.cpp @@ -1,8 +1,3 @@ -// constants for more human-readable format masks -const unsigned char HGEdge::simple = 1; -const unsigned char HGEdge::collapsed = 2; -const unsigned char HGEdge::traveled = 4; - HGEdge::HGEdge(HighwaySegment *s, HighwayGraph *graph) { // initial construction is based on a HighwaySegment s_written = 0; // simple diff --git a/siteupdate/cplusplus/classes/GraphGeneration/HGEdge.h b/siteupdate/cplusplus/classes/GraphGeneration/HGEdge.h index 7b6374e0..3f29ba3a 100644 --- a/siteupdate/cplusplus/classes/GraphGeneration/HGEdge.h +++ b/siteupdate/cplusplus/classes/GraphGeneration/HGEdge.h @@ -12,9 +12,9 @@ class HGEdge unsigned char format; // constants for more human-readable format masks - static const unsigned char simple; - static const unsigned char collapsed; - static const unsigned char traveled; + static const unsigned char simple = 1; + static const unsigned char collapsed = 2; + static const unsigned char traveled = 4; HGEdge(HighwaySegment *, HighwayGraph *); HGEdge(HGVertex *, unsigned char); diff --git a/siteupdate/cplusplus/classes/GraphGeneration/PlaceRadius.cpp b/siteupdate/cplusplus/classes/GraphGeneration/PlaceRadius.cpp index 63a4a77e..78b522a3 100644 --- a/siteupdate/cplusplus/classes/GraphGeneration/PlaceRadius.cpp +++ b/siteupdate/cplusplus/classes/GraphGeneration/PlaceRadius.cpp @@ -10,10 +10,10 @@ bool PlaceRadius::contains_vertex(HGVertex *v) {return contains_vertex(v->lat, v bool PlaceRadius::contains_vertex(double vlat, double vlng) { /* return whether coordinates are within this area */ // convert to radians to compute distance - double rlat1 = lat * (Waypoint::pi/180); - double rlng1 = lng * (Waypoint::pi/180); - double rlat2 = vlat * (Waypoint::pi/180); - double rlng2 = vlng * (Waypoint::pi/180); + double rlat1 = lat * (pi/180); + double rlng1 = lng * (pi/180); + double rlat2 = vlat * (pi/180); + double rlng2 = vlng * (pi/180); /* original formula double ans = acos(cos(rlat1)*cos(rlng1)*cos(rlat2)*cos(rlng2) +\ @@ -48,10 +48,10 @@ std::unordered_set PlaceRadius::vertices(WaypointQuadtree *qt, Highwa // N/S sanity check: If lat is <= r/2 miles to the N or S pole, lngdelta calculation will fail. // In these cases, our place radius will span the entire "width" of the world, from -180 to +180 degrees. - if (90-fabs(lat)*(Waypoint::pi/180) <= r/7926.2) return v_search(qt, g, -180, +180); + if (90-fabs(lat)*(pi/180) <= r/7926.2) return v_search(qt, g, -180, +180); // width, in degrees longitude, of our bounding box for quadtree search - double lngdelta = acos((cos(r/3963.1) - pow(sin(lat*(Waypoint::pi/180)),2)) / pow(cos(lat*(Waypoint::pi/180)),2)) / (Waypoint::pi/180); + double lngdelta = acos((cos(r/3963.1) - pow(sin(lat*(pi/180)),2)) / pow(cos(lat*(pi/180)),2)) / (pi/180); double w_bound = lng-lngdelta; double e_bound = lng+lngdelta; @@ -92,8 +92,8 @@ std::unordered_set PlaceRadius::v_search(WaypointQuadtree *qt, Highwa // if we're not a terminal quadrant, we need to determine which // of our child quadrants we need to search and recurse into each else { //printf("DEBUG: recursive case, mid_lat=%.17g mid_lng=%.17g\n", qt->mid_lat, qt->mid_lng); fflush(stdout); - bool look_n = (lat + r/3963.1/(Waypoint::pi/180)) >= qt->mid_lat; - bool look_s = (lat - r/3963.1/(Waypoint::pi/180)) <= qt->mid_lat; + bool look_n = (lat + r/3963.1/(pi/180)) >= qt->mid_lat; + bool look_s = (lat - r/3963.1/(pi/180)) <= qt->mid_lat; bool look_e = e_bound >= qt->mid_lng; bool look_w = w_bound <= qt->mid_lng; //std::cout << "DEBUG: recursive case, " << look_n << " " << look_s << " " << look_e << " " << look_w << std::endl; diff --git a/siteupdate/cplusplus/classes/HighwaySystem.cpp b/siteupdate/cplusplus/classes/HighwaySystem.cpp index fbdd9054..0d6bc2b3 100644 --- a/siteupdate/cplusplus/classes/HighwaySystem.cpp +++ b/siteupdate/cplusplus/classes/HighwaySystem.cpp @@ -79,7 +79,8 @@ class HighwaySystem if (!file) el.add_error("Could not open "+path+"/"+systemname+".csv"); else { getline(file, line); // ignore header line while(getline(file, line)) - { if (line.back() == 0x0D) line.erase(line.end()-1); // trim DOS newlines + { // trim DOS newlines & trailing whitespace + while ( strchr("\r\t ", line.back()) ) line.pop_back(); if (line.empty()) continue; route_list.emplace_back(line, this, el, region_hash); if (route_list.back().root.empty()) @@ -95,9 +96,10 @@ class HighwaySystem if (!file) el.add_error("Could not open "+path+"/"+systemname+"_con.csv"); else { getline(file, line); // ignore header line while(getline(file, line)) - { if (line.back() == 0x0D) line.erase(line.end()-1); // trim DOS newlines + { // trim DOS newlines & trailing whitespace + while ( strchr("\r\t ", line.back()) ) line.pop_back(); if (line.empty()) continue; - con_route_list.emplace_back(line, this, el, route_list); + con_route_list.emplace_back(line, this, el); } } file.close(); diff --git a/siteupdate/cplusplus/classes/Route/Route.cpp b/siteupdate/cplusplus/classes/Route/Route.cpp index 6c104d27..4cbf193c 100644 --- a/siteupdate/cplusplus/classes/Route/Route.cpp +++ b/siteupdate/cplusplus/classes/Route/Route.cpp @@ -1,3 +1,4 @@ +std::unordered_map Route::root_hash, Route::list_hash; std::mutex Route::awf_mtx; Route::Route(std::string &line, HighwaySystem *sys, ErrorList &el, std::unordered_map ®ion_hash) @@ -52,12 +53,28 @@ Route::Route(std::string &line, HighwaySystem *sys, ErrorList &el, std::unordere if (root.size() > DBFieldLength::root) el.add_error("Root > " + std::to_string(DBFieldLength::root) + " bytes in " + system->systemname + ".csv line: " + line); + lower(root.data()); // alt_route_names - size_t l = 0; - for (size_t r = 0; r != -1; l = r+1) - { r = arn_str.find(',', l); - alt_route_names.emplace_back(arn_str, l, r-l); + upper(arn_str.data()); + size_t len; + for (size_t pos = 0; pos < arn_str.size(); pos += len+1) + { len = strcspn(arn_str.data()+pos, ","); + alt_route_names.emplace_back(arn_str, pos, len); } + + // insert into root_hash, checking for duplicate root entries + if (!root_hash.insert(std::pair(root, this)).second) + el.add_error("Duplicate root in " + system->systemname + ".csv: " + root + + " already in " + root_hash.at(root)->system->systemname + ".csv"); + // insert into list_hash with list name, checking for duplicate .list names + if (!list_hash.insert(std::pair(upper(readable_name()), this)).second) + el.add_error("Duplicate main list name in " + root + ": '" + readable_name() + + "' already points to " + list_hash.at( upper(readable_name()) )->root); + // insert into list_hash with alt names, checking for duplicate .list names + for (std::string& a : alt_route_names) + if (!list_hash.insert(std::pair(upper(rg_str + " " + a), this)).second) + el.add_error("Duplicate alt route name in " + root + ": '" + region->code + ' ' + a + + "' already points to " + list_hash.at(upper(rg_str + " " + a))->root); } std::string Route::str() @@ -179,9 +196,3 @@ void Route::write_nmp_merged(std::string filename) } wptfile.close(); } - -Route *route_by_root(std::string root, std::list &route_list) -{ for (std::list::iterator r = route_list.begin(); r != route_list.end(); r++) - if (r->root == root) return &*r; - return 0; -} diff --git a/siteupdate/cplusplus/classes/Route/Route.h b/siteupdate/cplusplus/classes/Route/Route.h index c67a1479..7da12415 100644 --- a/siteupdate/cplusplus/classes/Route/Route.h +++ b/siteupdate/cplusplus/classes/Route/Route.h @@ -51,6 +51,7 @@ class Route std::vector point_list; std::unordered_set labels_in_use; std::unordered_set unused_alt_labels; + static std::unordered_map root_hash, list_hash; static std::mutex awf_mtx; // for locking the all_wpt_files set when erasing processed WPTs std::mutex liu_mtx; // for locking the labels_in_use set when inserting labels during TravelerList processing std::mutex ual_mtx; // for locking the unused_alt_labels set when removing in-use alt_labels diff --git a/siteupdate/cplusplus/classes/Route/read_wpt.cpp b/siteupdate/cplusplus/classes/Route/read_wpt.cpp index 433c025f..ac7817d3 100644 --- a/siteupdate/cplusplus/classes/Route/read_wpt.cpp +++ b/siteupdate/cplusplus/classes/Route/read_wpt.cpp @@ -63,16 +63,6 @@ void Route::read_wpt continue; } point_list.push_back(w); - // populate unused alt labels - for (size_t i = 0; i < w->alt_labels.size(); i++) - { std::string al = w->alt_labels[i]; - // strip out leading '+' - while (al[0] == '+') al.erase(0, 1); //TODO would erase via iterator be any faster? - // convert to upper case - for (size_t c = 0; c < al.size(); c++) - if (al[c] >= 'a' && al[c] <= 'z') al[c] -= 32; - unused_alt_labels.insert(al); - } all_waypoints->insert(w, 1); // single-point Datachecks, and HighwaySegment diff --git a/siteupdate/cplusplus/classes/TravelerList/TravelerList.cpp b/siteupdate/cplusplus/classes/TravelerList/TravelerList.cpp index e6419a35..72632136 100644 --- a/siteupdate/cplusplus/classes/TravelerList/TravelerList.cpp +++ b/siteupdate/cplusplus/classes/TravelerList/TravelerList.cpp @@ -29,7 +29,7 @@ class TravelerList unsigned int preview_systems_clinched; static std::mutex alltrav_mtx; // for locking the traveler_lists list when reading .lists from disk - TravelerList(std::string travname, std::unordered_map *route_hash, ErrorList *el, Arguments *args, std::mutex *strtok_mtx) + TravelerList(std::string travname, ErrorList *el, Arguments *args, std::mutex *strtok_mtx) { active_systems_traveled = 0; active_systems_clinched = 0; preview_systems_traveled = 0; @@ -116,11 +116,11 @@ class TravelerList } // find the root that matches in some system and when we do, match labels - std::string route_entry = lower(std::string(fields[1])); - std::string lookup = std::string(lower(fields[0])) + " " + route_entry; - try { Route *r = route_hash->at(lookup); - for (std:: string a : r->alt_route_names) - if (route_entry == lower(a)) + std::string route_entry = upper(std::string(fields[1])); // leave fields[1] intact for potential AltRouteName note + std::string lookup = std::string(upper(fields[0])) + " " + route_entry; + try { Route *r = Route::list_hash.at(lookup); + for (std::string& a : r->alt_route_names) + if (route_entry == a) { log << "Note: deprecated route name " << fields[1] << " -> canonical name " << r->list_entry_name() << " in line " << trim_line << '\n'; break; @@ -135,28 +135,28 @@ class TravelerList // "+" or "*" when matching std::vector point_indices; unsigned int checking_index = 0; + while (*fields[2] == '*' || *fields[2] == '+') fields[2]++; + while (*fields[3] == '*' || *fields[3] == '+') fields[3]++; + upper(fields[2]); + upper(fields[3]); for (Waypoint *w : r->point_list) - { std::string lower_label = lower(w->label); - while (*lower(fields[2]) == '*' || *fields[2] == '+') fields[2]++; - while (*lower(fields[3]) == '*' || *fields[3] == '+') fields[3]++; - while (lower_label.front() == '+' || lower_label.front() == '*') lower_label.erase(lower_label.begin()); - if (fields[2] == lower_label || fields[3] == lower_label) + { std::string upper_label = upper(w->label); + while (upper_label.front() == '+' || upper_label.front() == '*') upper_label = upper_label.substr(1); + if (fields[2] == upper_label || fields[3] == upper_label) { point_indices.push_back(checking_index); r->liu_mtx.lock(); - r->labels_in_use.insert(upper(lower_label)); + r->labels_in_use.insert(upper_label); r->liu_mtx.unlock(); } else { for (std::string &alt : w->alt_labels) - { lower_label = lower(alt); - while (lower_label.front() == '+') lower_label.erase(lower_label.begin()); - if (fields[2] == lower_label || fields[3] == lower_label) + { if (fields[2] == alt || fields[3] == alt) { point_indices.push_back(checking_index); r->liu_mtx.lock(); - r->labels_in_use.insert(upper(lower_label)); + r->labels_in_use.insert(alt); r->liu_mtx.unlock(); // if we have not yet used this alt label, remove it from the unused set r->ual_mtx.lock(); - r->unused_alt_labels.erase(upper(lower_label)); + r->unused_alt_labels.erase(alt); r->ual_mtx.unlock(); } } diff --git a/siteupdate/cplusplus/classes/Waypoint/Waypoint.cpp b/siteupdate/cplusplus/classes/Waypoint/Waypoint.cpp index 7b3bfe20..1b84ac51 100644 --- a/siteupdate/cplusplus/classes/Waypoint/Waypoint.cpp +++ b/siteupdate/cplusplus/classes/Waypoint/Waypoint.cpp @@ -13,8 +13,6 @@ bool waypoint_simplification_sort(Waypoint *w1, Waypoint *w2) else return 0; } -const double Waypoint::pi = 3.141592653589793238; - Waypoint::Waypoint(char *line, Route *rte, DatacheckEntryList *datacheckerrors) { /* initialize object from a .wpt file line */ route = rte; @@ -314,17 +312,14 @@ bool Waypoint::label_references_route(Route *r, DatacheckEntryList *datacheckerr inline void Waypoint::duplicate_label(DatacheckEntryList *datacheckerrors, std::unordered_set &all_route_labels) { // duplicate labels // first, check primary label - std::string lower_label = lower(label); - while (lower_label[0] == '+' || lower_label[0] == '*') lower_label.erase(lower_label.begin()); - if (!all_route_labels.insert(lower_label).second) - datacheckerrors->add(route, lower_label, "", "", "DUPLICATE_LABEL", ""); + std::string upper_label = upper(label); + while (upper_label[0] == '+' || upper_label[0] == '*') upper_label = upper_label.substr(1); + if (!all_route_labels.insert(upper_label).second) + datacheckerrors->add(route, upper_label, "", "", "DUPLICATE_LABEL", ""); // then check alt labels - for (std::string &label : alt_labels) - { std::string lower_label = lower(label); - while (lower_label[0] == '+' || lower_label[0] == '*') lower_label.erase(lower_label.begin()); - if (!all_route_labels.insert(lower_label).second) - datacheckerrors->add(route, lower_label, "", "", "DUPLICATE_LABEL", ""); - } + for (std::string &a : alt_labels) + if (!all_route_labels.insert(a).second) + datacheckerrors->add(route, a, "", "", "DUPLICATE_LABEL", ""); } inline void Waypoint::duplicate_coords(DatacheckEntryList *datacheckerrors, std::unordered_set &coords_used, char *fstr) diff --git a/siteupdate/cplusplus/classes/Waypoint/Waypoint.h b/siteupdate/cplusplus/classes/Waypoint/Waypoint.h index a419e8d3..e361800a 100644 --- a/siteupdate/cplusplus/classes/Waypoint/Waypoint.h +++ b/siteupdate/cplusplus/classes/Waypoint/Waypoint.h @@ -20,7 +20,6 @@ class Waypoint std::vector ap_coloc; std::forward_list near_miss_points; bool is_hidden; - static const double pi; Waypoint(char *, Route *, DatacheckEntryList *); diff --git a/siteupdate/cplusplus/functions/lower.cpp b/siteupdate/cplusplus/functions/lower.cpp index 228d0cde..1947d2c2 100644 --- a/siteupdate/cplusplus/functions/lower.cpp +++ b/siteupdate/cplusplus/functions/lower.cpp @@ -4,8 +4,8 @@ std::string lower(std::string str) return str; } -char *lower(char *str) -{ for (char *c = str; *c != 0; c++) +const char *lower(const char *str) +{ for (char* c = (char*)str; *c != 0; c++) if (*c >= 'A' && *c <= 'Z') *c += 32; return str; } diff --git a/siteupdate/cplusplus/functions/upper.cpp b/siteupdate/cplusplus/functions/upper.cpp index 31a5ae9d..883a1d07 100644 --- a/siteupdate/cplusplus/functions/upper.cpp +++ b/siteupdate/cplusplus/functions/upper.cpp @@ -3,3 +3,9 @@ std::string upper(std::string str) if (str[c] >= 'a' && str[c] <= 'z') str[c] -= 32; return str; } + +const char *upper(const char *str) +{ for (char* c = (char*)str; *c != 0; c++) + if (*c >= 'a' && *c <= 'z') *c -= 32; + return str; +} diff --git a/siteupdate/cplusplus/siteupdate.cpp b/siteupdate/cplusplus/siteupdate.cpp index cef22302..2cc92f38 100644 --- a/siteupdate/cplusplus/siteupdate.cpp +++ b/siteupdate/cplusplus/siteupdate.cpp @@ -25,6 +25,7 @@ class DatacheckEntryList; class HighwayGraph; class HGVertex; class HGEdge; +#define pi 3.141592653589793238 #include #include #include @@ -255,65 +256,6 @@ int main(int argc, char *argv[]) DatacheckEntryList *datacheckerrors = new DatacheckEntryList; // deleted on termination of program - // check for duplicate .list names - // and duplicate root entries among Route and ConnectedRoute - // data in all highway systems - { cout << et.et() << "Checking for duplicate list names in routes, roots in routes and connected routes." << endl; - unordered_set roots, list_names, duplicate_list_names; - unordered_set con_roots; - - for (HighwaySystem *h : highway_systems) - { for (Route& r : h->route_list) - { if (roots.find(r.root) == roots.end()) roots.insert(r.root); - else el.add_error("Duplicate root in route lists: " + r.root); - string list_name = r.readable_name(); - // FIXME branch based on result of list_names.insert - if (list_names.find(list_name) == list_names.end()) list_names.insert(list_name); - else duplicate_list_names.insert(list_name); - } - for (ConnectedRoute& cr : h->con_route_list) - for (size_t r = 0; r < cr.roots.size(); r++) - // FIXME branch based on result of list_names.insert - if (con_roots.find(cr.roots[r]) == con_roots.end()) con_roots.insert(cr.roots[r]); - else el.add_error("Duplicate root in con_route lists: " + cr.roots[r]->root);//*/ - } - - // Make sure every route was listed as a part of some connected route - if (roots.size() == con_roots.size()) - cout << "Check passed: same number of routes as connected route roots. " << roots.size() << endl; - else { el.add_error("Check FAILED: " + to_string(roots.size()) + " routes != " + to_string(con_roots.size()) + " connected route roots."); - // remove con_routes entries from roots - for (Route *cr : con_roots) - { unordered_set::iterator r = roots.find(cr->root); - if (r != roots.end()) roots.erase(r); //FIXME erase by value - } - // there will be some leftovers, let's look up their routes to make - // an error report entry (not worried about efficiency as there would - // only be a few in reasonable cases) - unsigned int num_found = 0; - for (HighwaySystem *h : highway_systems) - for (Route& r : h->route_list) - for (const string& lr : roots) - if (lr == r.root) - { el.add_error("route " + lr + " not matched by any connected route root."); - num_found++; - break; - } - cout << "Added " << num_found << " ROUTE_NOT_IN_CONNECTED error entries." << endl; - } - - // report any duplicate list names as errors - if (duplicate_list_names.empty()) cout << "No duplicate list names found." << endl; - else { for (string d : duplicate_list_names) el.add_error("Duplicate list name: " + d); - cout << "Added " << duplicate_list_names.size() << " DUPLICATE_LIST_NAME error entries." << endl; - } - - roots.clear(); - list_names.clear(); - duplicate_list_names.clear(); - con_roots.clear(); - } - // For tracking whether any .wpt files are in the directory tree // that do not have a .csv file entry that causes them to be // read into the data @@ -443,13 +385,17 @@ int main(int argc, char *argv[]) #include "functions/concurrency_detection.cpp" - // Create hash table for faster lookup of routes by list file name - cout << et.et() << "Creating route hash table for list processing:" << endl; - unordered_map route_hash; - for (HighwaySystem *h : highway_systems) - for (list::iterator r = h->route_list.begin(); r != h->route_list.end(); r++) //FIXME use more compact syntax (&) - { route_hash[lower(r->readable_name())] = &*r; - for (string &a : r->alt_route_names) route_hash[lower(r->rg_str + " " + a)] = &*r; + cout << et.et() << "Checking for unconnected chopped routes, creating canonical AltLabels & populating unused sets." << endl; + for (HighwaySystem* h : highway_systems) + for (Route& r : h->route_list) + { if (!r.con_route) + el.add_error(r.system->systemname + ".csv: root " + r.root + " not matched by any connected route root."); + for (Waypoint* w : r.point_list) + for (std::string& a : w->alt_labels) + { while (a[0] == '+' || a[0] == '*') a = a.substr(1); + upper(a.data()); + r.unused_alt_labels.insert(a); + } } // Create a list of TravelerList objects, one per person @@ -460,7 +406,7 @@ int main(int argc, char *argv[]) #ifdef threading_enabled // set up for threaded .list file processing for (unsigned int t = 0; t < args.numthreads; t++) - thr[t] = new thread(ReadListThread, t, &traveler_ids, &id_it, &traveler_lists, &list_mtx, &strtok_mtx, &args, &route_hash, &el); + thr[t] = new thread(ReadListThread, t, &traveler_ids, &id_it, &traveler_lists, &list_mtx, &strtok_mtx, &args, &el); for (unsigned int t = 0; t < args.numthreads; t++) thr[t]->join(); for (unsigned int t = 0; t < args.numthreads; t++) @@ -468,7 +414,7 @@ int main(int argc, char *argv[]) #else for (string &t : traveler_ids) { cout << t << ' ' << std::flush; - traveler_lists.push_back(new TravelerList(t, &route_hash, &el, &args, &strtok_mtx)); + traveler_lists.push_back(new TravelerList(t, &el, &args, &strtok_mtx)); } traveler_ids.clear(); #endif diff --git a/siteupdate/cplusplus/threads/ReadListThread.cpp b/siteupdate/cplusplus/threads/ReadListThread.cpp index 5adaa85c..923fb45b 100644 --- a/siteupdate/cplusplus/threads/ReadListThread.cpp +++ b/siteupdate/cplusplus/threads/ReadListThread.cpp @@ -1,7 +1,7 @@ void ReadListThread ( unsigned int id, std::list *traveler_ids, std::list::iterator *it, std::list *traveler_lists, std::mutex *tl_mtx, std::mutex *strtok_mtx, - Arguments *args, std::unordered_map *route_hash, ErrorList *el + Arguments *args, ErrorList *el ) { //printf("Starting ReadListThread %02i\n", id); fflush(stdout); while (*it != traveler_ids->end()) @@ -16,7 +16,7 @@ void ReadListThread //printf("ReadListThread %02i (*it)++\n", id); fflush(stdout); std::cout << tl << ' ' << std::flush; tl_mtx->unlock(); - TravelerList *t = new TravelerList(tl, route_hash, el, args, strtok_mtx); + TravelerList *t = new TravelerList(tl, el, args, strtok_mtx); // deleted on termination of program TravelerList::alltrav_mtx.lock(); traveler_lists->push_back(t); diff --git a/siteupdate/python-teresco/siteupdate.py b/siteupdate/python-teresco/siteupdate.py index 9e764491..873ada68 100755 --- a/siteupdate/python-teresco/siteupdate.py +++ b/siteupdate/python-teresco/siteupdate.py @@ -894,6 +894,9 @@ class Route: AltRouteNames: (optional) comma-separated list former or other alternate route names that might appear in user list files. """ + root_hash = dict() + list_hash = dict() + def __init__(self,line,system,el): """initialize object from a .csv file line, but do not yet read in waypoint file""" @@ -936,16 +939,43 @@ def __init__(self,line,system,el): el.add_error("City > " + str(DBFieldLength.city) + " bytes in " + system.systemname + ".csv line: " + line) - self.root = fields[6] + self.root = fields[6].lower() if len(self.root.encode('utf-8')) > DBFieldLength.root: el.add_error("Root > " + str(DBFieldLength.root) + " bytes in " + system.systemname + ".csv line: " + line) - self.alt_route_names = fields[7].split(",") + if len(fields[7]) == 0: + self.alt_route_names = [] + else: + self.alt_route_names = fields[7].upper().split(",") + + # insert into root_hash, checking for duplicate root entries + if self.root in Route.root_hash: + el.add_error("Duplicate root in " + system.systemname + ".csv: " + self.root + + " already in " + Route.root_hash[self.root].system.systemname + ".csv") + else: + Route.root_hash[self.root] = self + # insert into list_hash with list name, checking for duplicate .list names + list_name = self.readable_name().upper() + if list_name in Route.list_hash: + el.add_error("Duplicate main list name in " + self.root + ": '" + self.readable_name() + + "' already points to " + Route.list_hash[list_name].root) + else: + Route.list_hash[list_name] = self + # insert into list_hash with alt names, checking for duplicate .list names + for a in self.alt_route_names: + list_name = self.region.upper() + ' ' + a + if list_name in Route.list_hash: + el.add_error("Duplicate alt route name in " + self.root + ": '" + self.region + ' ' + a + + "' already points to " + Route.list_hash[list_name].root) + else: + Route.list_hash[list_name] = self + self.point_list = [] self.labels_in_use = set() self.unused_alt_labels = set() self.segment_list = [] + self.con_route = None self.mileage = 0.0 self.rootOrder = -1 # order within connected route @@ -976,9 +1006,6 @@ def read_wpt(self,all_waypoints,all_waypoints_lock,datacheckerrors,el,path="../. w = previous_point continue self.point_list.append(w) - # populate unused alt labels - for label in w.alt_labels: - self.unused_alt_labels.add(label.upper().strip("+")) all_waypoints_lock.acquire() # look for near-miss points (before we add this one in) @@ -1086,22 +1113,24 @@ def __init__(self,line,system,el): "_con.csv line: " + line) # fields[4] is the list of roots, which will become a python list # of Route objects already in the system - roots = fields[4].split(",") rootOrder = 0 - for root in roots: - route = None - for check_route in system.route_list: - if check_route.root == root: - route = check_route - break - if route is None: - el.add_error("Could not find Route matching ConnectedRoute root " + root + - " in system " + system.systemname + '.') - else: + for root in fields[4].lower().split(","): + try: + route = Route.root_hash[root] self.roots.append(route) + if route.con_route is not None: + el.add_error("Duplicate root in " + system.systemname + "_con.csv: " + route.root + + " already in " + route.con_route.system.systemname + "_con.csv") + if system != route.system: + el.add_error("System mismatch: chopped route " + route.root + " from " + route.system.systemname + + ".csv in connected route in " + system.systemname + "_con.csv"); + route.con_route = self # save order of route in connected route route.rootOrder = rootOrder - rootOrder += 1 + rootOrder += 1 + except KeyError: + el.add_error("Could not find Route matching ConnectedRoute root " + root + + " in system " + system.systemname + '.') if len(self.roots) < 1: el.add_error("No roots in " + system.systemname + "_con.csv line: " + line) # will be computed for routes in active & preview systems later @@ -1214,7 +1243,7 @@ class TravelerList: start_waypoint end_waypoint """ - def __init__(self,travelername,route_hash,el,path="../../../UserData/list_files"): + def __init__(self,travelername,el,path="../../../UserData/list_files"): list_entries = 0 self.clinched_segments = set() self.traveler_name = travelername[:-5] @@ -1240,17 +1269,17 @@ def __init__(self,travelername,route_hash,el,path="../../../UserData/list_files" continue # find the root that matches in some system and when we do, match labels - route_entry = fields[1].lower() - lookup = fields[0].lower() + ' ' + route_entry - if lookup not in route_hash: + route_entry = fields[1].upper() + lookup = fields[0].upper() + ' ' + route_entry + if lookup not in Route.list_hash: (line, invchar) = no_control_chars(line) self.log_entries.append("Unknown region/highway combo in line: " + line) if invchar: self.log_entries[-1] += " [contains invalid character(s)]" else: - r = route_hash[lookup] + r = Route.list_hash[lookup] for a in r.alt_route_names: - if route_entry == a.lower(): + if route_entry == a: self.log_entries.append("Note: deprecated route name " + fields[1] + " -> canonical name " + r.list_entry_name() + " in line " + line) break @@ -1262,22 +1291,19 @@ def __init__(self,travelername,route_hash,el,path="../../../UserData/list_files" # "+" or "*" when matching point_indices = [] checking_index = 0; + list_label_1 = fields[2].lstrip("+*").upper() + list_label_2 = fields[3].lstrip("+*").upper() for w in r.point_list: - lower_label = w.label.lower().lstrip("+*") - list_label_1 = fields[2].lower().lstrip("+*") - list_label_2 = fields[3].lower().lstrip("+*") - if list_label_1 == lower_label or list_label_2 == lower_label: + upper_label = w.label.lstrip("+*").upper() + if list_label_1 == upper_label or list_label_2 == upper_label: point_indices.append(checking_index) - r.labels_in_use.add(lower_label.upper()) + r.labels_in_use.add(upper_label) else: for alt in w.alt_labels: - lower_label = alt.lower().strip("+") - if list_label_1 == lower_label or list_label_2 == lower_label: + if list_label_1 == alt or list_label_2 == alt: point_indices.append(checking_index) - r.labels_in_use.add(lower_label.upper()) - # if we have not yet used this alt label, remove it from the unused list - if lower_label.upper() in r.unused_alt_labels: - r.unused_alt_labels.remove(lower_label.upper()) + r.labels_in_use.add(alt) + r.unused_alt_labels.discard(alt) checking_index += 1 if len(point_indices) != 2: @@ -2426,69 +2452,6 @@ def __init__(self,filename,descr,vertices,edges,travelers,format,category): # list for datacheck errors that we will need later datacheckerrors = [] -# check for duplicate root entries among Route and ConnectedRoute -# data in all highway systems -print(et.et() + "Checking for duplicate list names in routes, roots in routes and connected routes.",flush=True) -roots = set() -list_names = set() -duplicate_list_names = set() -for h in highway_systems: - for r in h.route_list: - if r.root in roots: - el.add_error("Duplicate root in route lists: " + r.root) - else: - roots.add(r.root) - list_name = r.region + ' ' + r.list_entry_name() - if list_name in list_names: - duplicate_list_names.add(list_name) - else: - list_names.add(list_name) - -con_roots = set() -for h in highway_systems: - for cr in h.con_route_list: - for r in cr.roots: - if r.root in con_roots: - el.add_error("Duplicate root in con_route lists: " + r.root) - else: - con_roots.add(r.root) - -# Make sure every route was listed as a part of some connected route -if len(roots) == len(con_roots): - print("Check passed: same number of routes as connected route roots. " + str(len(roots))) -else: - el.add_error("Check FAILED: " + str(len(roots)) + " routes != " + str(len(con_roots)) + " connected route roots.") - roots = roots - con_roots - # there will be some leftovers, let's look up their routes to make - # an error report entry (not worried about efficiency as there would - # only be a few in reasonable cases) - num_found = 0 - for h in highway_systems: - for r in h.route_list: - for lr in roots: - if lr == r.root: - el.add_error("route " + lr + " not matched by any connected route root.") - num_found += 1 - break - print("Added " + str(num_found) + " ROUTE_NOT_IN_CONNECTED error entries.") - -# report any duplicate list names as errors -if len(duplicate_list_names) > 0: - print("Found " + str(len(duplicate_list_names)) + " DUPLICATE_LIST_NAME case(s).") - for d in duplicate_list_names: - el.add_error("Duplicate list name: " + d) -else: - print("No duplicate list names found.") - -# write file mapping CHM datacheck route lists to root (commented out, -# unlikely needed now) -#print(et.et() + "Writing CHM datacheck to TravelMapping route pairings.") -#file = open(args.csvstatfilepath + "/routepairings.csv","wt") -#for h in highway_systems: -# for r in h.route_list: -# file.write(r.region + " " + r.list_entry_name() + ";" + r.root + "\n") -#file.close() - # For tracking whether any .wpt files are in the directory tree # that do not have a .csv file entry that causes them to be # read into the data @@ -2718,14 +2681,15 @@ def run(self): print(".", end="", flush=True) print() -# Create hash table for faster lookup of routes by list file name -print(et.et() + "Creating route hash table for list processing:",flush=True) -route_hash = dict() +print(et.et() + "Checking for unconnected chopped routes, creating canonical AltLabels & populating unused sets.",flush=True) for h in highway_systems: for r in h.route_list: - route_hash[(r.region + ' ' + r.list_entry_name()).lower()] = r - for a in r.alt_route_names: - route_hash[(r.region + ' ' + a).lower()] = r + if r.con_route is None: + el.add_error(r.system.systemname + ".csv: root " + r.root + " not matched by any connected route root.") + for w in r.point_list: + for a in range(len(w.alt_labels)): + w.alt_labels[a] = w.alt_labels[a].lstrip('+*').upper() + r.unused_alt_labels.add(w.alt_labels[a]) # Create a list of TravelerList objects, one per person traveler_lists = [] @@ -2734,7 +2698,7 @@ def run(self): for t in traveler_ids: if t.endswith('.list'): print(t + " ",end="",flush=True) - traveler_lists.append(TravelerList(t,route_hash,el,args.userlistfilepath)) + traveler_lists.append(TravelerList(t,el,args.userlistfilepath)) print('\n' + et.et() + "Processed " + str(len(traveler_lists)) + " traveler list files.") traveler_lists.sort(key=lambda TravelerList: TravelerList.traveler_name) # assign traveler numbers @@ -3605,18 +3569,17 @@ def run(self): for w in r.point_list: # duplicate labels # first, check primary label - lower_label = w.label.lower().strip("+*") - if lower_label in all_route_labels: - datacheckerrors.append(DatacheckEntry(r,[lower_label],"DUPLICATE_LABEL")) + upper_label = w.label.strip("+*").upper() + if upper_label in all_route_labels: + datacheckerrors.append(DatacheckEntry(r,[upper_label],"DUPLICATE_LABEL")) else: - all_route_labels.add(lower_label) + all_route_labels.add(upper_label) # then check alt labels - for label in w.alt_labels: - lower_label = label.lower().strip("+*") - if lower_label in all_route_labels: - datacheckerrors.append(DatacheckEntry(r,[lower_label],"DUPLICATE_LABEL")) + for a in w.alt_labels: + if a in all_route_labels: + datacheckerrors.append(DatacheckEntry(r,[a],"DUPLICATE_LABEL")) else: - all_route_labels.add(lower_label) + all_route_labels.add(a) # out-of-bounds coords if w.lat > 90 or w.lat < -90 or w.lng > 180 or w.lng < -180: From f50764d0ee137d66ff32a543c00ff2260d6bf0c0 Mon Sep 17 00:00:00 2001 From: eric bryant Date: Wed, 13 May 2020 23:03:35 -0400 Subject: [PATCH 2/2] range for loop cleanup * Retrieve elements by value rather than reference. Same principle as in #270, just without the big speed increase. Best practice nonetheless. * Convert traditional for loops to range for loops where possible, for cleaner more concise code. --- .../classes/GraphGeneration/HighwayGraph.cpp | 4 +- .../classes/HighwaySegment/HighwaySegment.cpp | 16 +++--- .../cplusplus/classes/HighwaySystem.cpp | 2 +- siteupdate/cplusplus/classes/Region.cpp | 4 +- .../classes/TravelerList/TravelerList.cpp | 18 +++---- .../WaypointQuadtree/WaypointQuadtree.cpp | 4 +- .../cplusplus/functions/graph_generation.cpp | 5 +- siteupdate/cplusplus/functions/sql_file.cpp | 4 +- siteupdate/cplusplus/siteupdate.cpp | 51 +++++++++---------- 9 files changed, 52 insertions(+), 56 deletions(-) diff --git a/siteupdate/cplusplus/classes/GraphGeneration/HighwayGraph.cpp b/siteupdate/cplusplus/classes/GraphGeneration/HighwayGraph.cpp index 8ae6bfaf..238d9d6b 100644 --- a/siteupdate/cplusplus/classes/GraphGeneration/HighwayGraph.cpp +++ b/siteupdate/cplusplus/classes/GraphGeneration/HighwayGraph.cpp @@ -96,7 +96,7 @@ class HighwayGraph // compress edges adjacent to hidden vertices counter = 0; std::cout << et.et() + "Compressing collapsed edges" << std::flush; - for (std::pair wv : vertices) + for (std::pair& wv : vertices) { if (counter % 10000 == 0) std::cout << '.' << std::flush; counter++; if (!wv.second->visibility) @@ -146,7 +146,7 @@ class HighwayGraph } // end ctor void clear() - { for (std::pair wv : vertices) delete wv.second; + { for (std::pair& wv : vertices) delete wv.second; vertex_names.clear(); waypoint_naming_log.clear(); vertices.clear(); diff --git a/siteupdate/cplusplus/classes/HighwaySegment/HighwaySegment.cpp b/siteupdate/cplusplus/classes/HighwaySegment/HighwaySegment.cpp index 30cccdf1..4b5d009f 100644 --- a/siteupdate/cplusplus/classes/HighwaySegment/HighwaySegment.cpp +++ b/siteupdate/cplusplus/classes/HighwaySegment/HighwaySegment.cpp @@ -49,19 +49,15 @@ unsigned int HighwaySegment::index() /*std::string HighwaySegment::concurrent_travelers_sanity_check() { if (route->system->devel()) return ""; if (concurrent) - for (HighwaySegment *conc : *concurrent) - { if (clinched_by.size() != conc->clinched_by.size()) - { if (conc->route->system->devel()) continue; + for (HighwaySegment *other : *concurrent) + { if (clinched_by.size() != other->clinched_by.size()) + { if (other->route->system->devel()) continue; return "[" + str() + "] clinched by " + std::to_string(clinched_by.size()) + " travelers; [" \ - + conc->str() + "] clinched by " + std::to_string(conc->clinched_by.size()) + '\n'; + + other->str() + "] clinched by " + std::to_string(other->clinched_by.size()) + '\n'; } else for (TravelerList *t : clinched_by) - { std::list::iterator ct; - for (ct = conc->clinched_by.begin(); ct != conc->clinched_by.end(); ct++) - if (*ct == t) break; - if (ct == conc->clinched_by.end()) - return t->traveler_name + " has clinched [" + str() + "], but not [" + conc->str() + "]\n"; - } + if (other->clinched_by.find(t) == other->clinched_by.end()) + return t->traveler_name + " has clinched [" + str() + "], but not [" + other->str() + "]\n"; } return ""; }//*/ diff --git a/siteupdate/cplusplus/classes/HighwaySystem.cpp b/siteupdate/cplusplus/classes/HighwaySystem.cpp index 0d6bc2b3..2ee68d96 100644 --- a/siteupdate/cplusplus/classes/HighwaySystem.cpp +++ b/siteupdate/cplusplus/classes/HighwaySystem.cpp @@ -128,7 +128,7 @@ class HighwaySystem /* Return total system mileage across all regions */ double total_mileage() { double mi = 0; - for (std::pair rm : mileage_by_region) mi += rm.second; + for (std::pair& rm : mileage_by_region) mi += rm.second; return mi; } diff --git a/siteupdate/cplusplus/classes/Region.cpp b/siteupdate/cplusplus/classes/Region.cpp index 4716aa2e..d7daa493 100644 --- a/siteupdate/cplusplus/classes/Region.cpp +++ b/siteupdate/cplusplus/classes/Region.cpp @@ -1,6 +1,6 @@ std::pair *country_or_continent_by_code(std::string code, std::vector> &pair_vector) -{ for (std::vector>::iterator c = pair_vector.begin(); c != pair_vector.end(); c++) - if (c->first == code) return &*c; +{ for (std::pair& c : pair_vector) + if (c.first == code) return &c; return 0; } diff --git a/siteupdate/cplusplus/classes/TravelerList/TravelerList.cpp b/siteupdate/cplusplus/classes/TravelerList/TravelerList.cpp index 72632136..d2a9ded1 100644 --- a/siteupdate/cplusplus/classes/TravelerList/TravelerList.cpp +++ b/siteupdate/cplusplus/classes/TravelerList/TravelerList.cpp @@ -165,9 +165,9 @@ class TravelerList } if (point_indices.size() != 2) { bool invalid_char = 0; - for (size_t c = 0; c < trim_line.size(); c++) - if (iscntrl(trim_line[c])) - { trim_line[c] = '?'; + for (char& c : trim_line) + if (iscntrl(c)) + { c = '?'; invalid_char = 1; } log << "Waypoint label(s) not found in line: " << trim_line; @@ -189,9 +189,9 @@ class TravelerList } catch (const std::out_of_range& oor) { bool invalid_char = 0; - for (size_t c = 0; c < trim_line.size(); c++) - if (iscntrl(trim_line[c])) - { trim_line[c] = '?'; + for (char& c : trim_line) + if (iscntrl(c)) + { c = '?'; invalid_char = 1; } log << "Unknown region/highway combo in line: " << trim_line; @@ -209,21 +209,21 @@ class TravelerList /* Return active mileage across all regions */ double active_only_miles() { double mi = 0; - for (std::pair rm : active_only_mileage_by_region) mi += rm.second; + for (std::pair& rm : active_only_mileage_by_region) mi += rm.second; return mi; } /* Return active+preview mileage across all regions */ double active_preview_miles() { double mi = 0; - for (std::pair rm : active_preview_mileage_by_region) mi += rm.second; + for (std::pair& rm : active_preview_mileage_by_region) mi += rm.second; return mi; } /* Return mileage across all regions for a specified system */ double system_region_miles(HighwaySystem *h) { double mi = 0; - for (std::pair rm : system_region_mileages.at(h)) mi += rm.second; + for (std::pair& rm : system_region_mileages.at(h)) mi += rm.second; return mi; } diff --git a/siteupdate/cplusplus/classes/WaypointQuadtree/WaypointQuadtree.cpp b/siteupdate/cplusplus/classes/WaypointQuadtree/WaypointQuadtree.cpp index b6285089..e6e86243 100644 --- a/siteupdate/cplusplus/classes/WaypointQuadtree/WaypointQuadtree.cpp +++ b/siteupdate/cplusplus/classes/WaypointQuadtree/WaypointQuadtree.cpp @@ -259,7 +259,7 @@ void WaypointQuadtree::write_qt_tmg(std::string filename) std::ofstream tmgfile(filename); tmgfile << "TMG 1.0 simple\n"; tmgfile << vertices.size() << ' ' << edges.size() << '\n'; - for (std::string v : vertices) tmgfile << v << '\n'; - for (std::string e : edges) tmgfile << e << '\n'; + for (std::string& v : vertices) tmgfile << v << '\n'; + for (std::string& e : edges) tmgfile << e << '\n'; tmgfile.close(); } diff --git a/siteupdate/cplusplus/functions/graph_generation.cpp b/siteupdate/cplusplus/functions/graph_generation.cpp index 02b3e0d6..9b47f0f3 100644 --- a/siteupdate/cplusplus/functions/graph_generation.cpp +++ b/siteupdate/cplusplus/functions/graph_generation.cpp @@ -5,7 +5,7 @@ HighwayGraph graph_data(all_waypoints, highway_systems, datacheckerrors, args.nu cout << et.et() << "Writing graph waypoint simplification log." << endl; ofstream wslogfile(args.logfilepath + "/waypointsimplification.log"); -for (string line : graph_data.waypoint_naming_log) +for (string& line : graph_data.waypoint_naming_log) wslogfile << line << '\n'; wslogfile.close(); graph_data.waypoint_naming_log.clear(); @@ -34,7 +34,8 @@ else { list *regions; graph_vector[0].edges = 0; graph_vector[0].vertices = graph_data.vertices.size(); graph_vector[1].edges = 0; graph_vector[1].vertices = 0; graph_vector[2].edges = 0; graph_vector[2].vertices = 0; - for (std::pair wv : graph_data.vertices) + // get master graph vertex & edge counts for terminal output before writing files + for (std::pair& wv : graph_data.vertices) { graph_vector[0].edges += wv.second->incident_s_edges.size(); if (wv.second->visibility >= 1) { graph_vector[2].vertices++; diff --git a/siteupdate/cplusplus/functions/sql_file.cpp b/siteupdate/cplusplus/functions/sql_file.cpp index ac9c05d0..2317eefb 100644 --- a/siteupdate/cplusplus/functions/sql_file.cpp +++ b/siteupdate/cplusplus/functions/sql_file.cpp @@ -273,7 +273,7 @@ void sqlfile1 first = 1; for (HighwaySystem *h : *highway_systems) if (h->active_or_preview()) - for (std::pair rm : h->mileage_by_region) + for (std::pair& rm : h->mileage_by_region) { if (!first) sqlfile << ','; first = 0; char fstr[35]; @@ -293,7 +293,7 @@ void sqlfile1 sqlfile << "INSERT INTO clinchedOverallMileageByRegion VALUES\n"; first = 1; for (TravelerList *t : *traveler_lists) - for (std::pair rm : t->active_preview_mileage_by_region) + for (std::pair& rm : t->active_preview_mileage_by_region) { if (!first) sqlfile << ','; first = 0; double active_miles = 0; diff --git a/siteupdate/cplusplus/siteupdate.cpp b/siteupdate/cplusplus/siteupdate.cpp index 2cc92f38..7ab1b1d1 100644 --- a/siteupdate/cplusplus/siteupdate.cpp +++ b/siteupdate/cplusplus/siteupdate.cpp @@ -248,7 +248,7 @@ int main(int argc, char *argv[]) } cout << endl; // at the end, print the lines ignored - for (string l : ignoring) cout << l << endl; + for (string& l : ignoring) cout << l << endl; ignoring.clear(); } file.close(); @@ -285,9 +285,8 @@ int main(int argc, char *argv[]) #else for (HighwaySystem* h : highway_systems) { std::cout << h->systemname << std::flush; - for (std::list::iterator r = h->route_list.begin(); r != h->route_list.end(); r++) - { r->read_wpt(&all_waypoints, &el, args.highwaydatapath+"/hwy_data", datacheckerrors, &all_wpt_files); - } + for (Route& r : h->route_list) + r.read_wpt(&all_waypoints, &el, args.highwaydatapath+"/hwy_data", datacheckerrors, &all_wpt_files); std::cout << "!" << std::endl; } #endif @@ -529,7 +528,7 @@ int main(int argc, char *argv[]) { inusefile << r.root << '(' << r.point_list.size() << "):"; list liu_list(r.labels_in_use.begin(), r.labels_in_use.end()); liu_list.sort(); - for (string label : liu_list) inusefile << ' ' << label; + for (string& label : liu_list) inusefile << ' ' << label; inusefile << '\n'; r.labels_in_use.clear(); } @@ -546,7 +545,7 @@ int main(int argc, char *argv[]) string ual_entry = r.root + '(' + to_string(r.unused_alt_labels.size()) + "):"; list ual_list(r.unused_alt_labels.begin(), r.unused_alt_labels.end()); ual_list.sort(); - for (string label : ual_list) ual_entry += ' ' + label; + for (string& label : ual_list) ual_entry += ' ' + label; r.unused_alt_labels.clear(); unused_alt_labels.push_back(ual_entry); } @@ -656,7 +655,7 @@ int main(int argc, char *argv[]) region_entries.push_back(region->code + fstr); } region_entries.sort(); - for (string e : region_entries) hdstatsfile << e; + for (string& e : region_entries) hdstatsfile << e; for (HighwaySystem *h : highway_systems) { sprintf(fstr, ") total: %.2f mi\n", h->total_mileage()); @@ -664,7 +663,7 @@ int main(int argc, char *argv[]) if (h->mileage_by_region.size() > 1) { hdstatsfile << "System " << h->systemname << " by region:\n"; list regions_in_system; - for (pair rm : h->mileage_by_region) + for (pair& rm : h->mileage_by_region) regions_in_system.push_back(rm.first); regions_in_system.sort(sort_regions_by_code); for (Region *r : regions_in_system) @@ -673,19 +672,19 @@ int main(int argc, char *argv[]) } } hdstatsfile << "System " << h->systemname << " by route:\n"; - for (list::iterator cr = h->con_route_list.begin(); cr != h->con_route_list.end(); cr++) + for (ConnectedRoute& cr : h->con_route_list) { double con_total_miles = 0; string to_write = ""; - for (Route *r : cr->roots) + for (Route *r : cr.roots) { sprintf(fstr, ": %.2f mi\n", r->mileage); to_write += " " + r->readable_name() + fstr; con_total_miles += r->mileage; } - cr->mileage = con_total_miles; //FIXME? + cr.mileage = con_total_miles; //FIXME? sprintf(fstr, ": %.2f mi", con_total_miles); - hdstatsfile << cr->readable_name() << fstr; - if (cr->roots.size() == 1) - hdstatsfile << " (" << cr->roots[0]->readable_name() << " only)\n"; + hdstatsfile << cr.readable_name() << fstr; + if (cr.roots.size() == 1) + hdstatsfile << " (" << cr.roots[0]->readable_name() << " only)\n"; else hdstatsfile << '\n' << to_write; } } @@ -733,7 +732,7 @@ int main(int argc, char *argv[]) allfile << '\n'; for (TravelerList *t : traveler_lists) { double t_total_mi = 0; - for (std::pair rm : t->active_only_mileage_by_region) + for (std::pair& rm : t->active_only_mileage_by_region) t_total_mi += rm.second; sprintf(fstr, "%.2f", t_total_mi); allfile << t->traveler_name << ',' << fstr; @@ -771,7 +770,7 @@ int main(int argc, char *argv[]) allfile << '\n'; for (TravelerList *t : traveler_lists) { double t_total_mi = 0; - for (std::pair rm : t->active_preview_mileage_by_region) + for (std::pair& rm : t->active_preview_mileage_by_region) t_total_mi += rm.second; sprintf(fstr, "%.2f", t_total_mi); allfile << t->traveler_name << ',' << fstr; @@ -799,7 +798,7 @@ int main(int argc, char *argv[]) sysfile << "Traveler,Total"; regions.clear(); total_mi = 0; - for (std::pair rm : h->mileage_by_region) + for (std::pair& rm : h->mileage_by_region) { regions.push_back(rm.first); total_mi += rm.second; //TODO is this right? } @@ -835,7 +834,7 @@ int main(int argc, char *argv[]) cout << et.et() << "Reading datacheckfps.csv." << endl; file.open(args.highwaydatapath+"/datacheckfps.csv"); getline(file, line); // ignore header line - list> datacheckfps; //FIXME try implementing as an unordered_multiset; see if speed increases + list> datacheckfps; unordered_set datacheck_always_error ({ "BAD_ANGLE", "DUPLICATE_LABEL", "HIDDEN_TERMINUS", "INVALID_FINAL_CHAR", "INVALID_FIRST_CHAR", @@ -881,22 +880,22 @@ int main(int argc, char *argv[]) fpfile << "Log file created at: " << ctime(×tamp); unsigned int counter = 0; unsigned int fpcount = 0; - for (list::iterator d = datacheckerrors->entries.begin(); d != datacheckerrors->entries.end(); d++) + for (DatacheckEntry& d : datacheckerrors->entries) { //cout << "Checking: " << d->str() << endl; counter++; if (counter % 1000 == 0) cout << '.' << flush; for (list>::iterator fp = datacheckfps.begin(); fp != datacheckfps.end(); fp++) - if (d->match_except_info(*fp)) - if (d->info == (*fp)[5]) + if (d.match_except_info(*fp)) + if (d.info == (*fp)[5]) { //cout << "Match!" << endl; - d->fp = 1; + d.fp = 1; fpcount++; datacheckfps.erase(fp); break; } else { fpfile << "FP_ENTRY: " << (*fp)[0] << ';' << (*fp)[1] << ';' << (*fp)[2] << ';' << (*fp)[3] << ';' << (*fp)[4] << ';' << (*fp)[5] << '\n'; - fpfile << "CHANGETO: " << (*fp)[0] << ';' << (*fp)[1] << ';' << (*fp)[2] << ';' << (*fp)[3] << ';' << (*fp)[4] << ';' << d->info << '\n'; + fpfile << "CHANGETO: " << (*fp)[0] << ';' << (*fp)[1] << ';' << (*fp)[2] << ';' << (*fp)[3] << ';' << (*fp)[4] << ';' << d.info << '\n'; } } fpfile.close(); @@ -910,8 +909,8 @@ int main(int argc, char *argv[]) timestamp = time(0); fpfile << "Log file created at: " << ctime(×tamp); if (datacheckfps.empty()) fpfile << "No unmatched FP entries.\n"; - else for (array entry : datacheckfps) - fpfile << entry[0] << ';' << entry[1] << ';' << entry[2] << ';' << entry[3] << ';' << entry[4] << ';' << entry[5] << '\n'; + else for (array& entry : datacheckfps) + fpfile << entry[0] << ';' << entry[1] << ';' << entry[2] << ';' << entry[3] << ';' << entry[4] << ';' << entry[5] << '\n'; fpfile.close(); // datacheck.log file @@ -999,7 +998,7 @@ int main(int argc, char *argv[]) unsigned other_count = 0; unsigned int total_rtes = 0; for (HighwaySystem *h : highway_systems) - for (Route r : h->route_list) + for (Route& r : h->route_list) { total_rtes++; if (h->devel()) d_count++; else { if (h->active()) a_count++;