Skip to content

making better use of std::unordered map #216

Description

@yakra

Looking at DBFieldLength.h for #119, I starting looking into when to use constexpr, which led me here, then here.

My personal recommendation is to avoid the [] operator. If you want to use an entry if it exists, then use map::find().

auto item = m.find(2);
if (item != m.end()) {
DoSomethingWith(*item);
}

Reminded me that a lot of the time I compare the result of std::unordered_map::find to std::unordered_map::end ignore it when I could be saving it to a variable, and then waste time doing a lookup again later, usually with std::unordered_map::at.

That article led me to this video. Neato. I'd noticed the behavior the presenter describes, but avoided it due to the same "newbie" expectations he cites, of uninitialized rather than value-initialized doubles. Thought it was possible that not every C++ compiler would gimme the results I was seeing. Great news. This could mean good things for compute_stats_t.
Finally, the first of thesetwo StackOverflow threads leads right back to Louis Brandy's video.

There are several ways to better use std::unordered map:

catch

Consider using std::unordered_map::find, comparing to std::unordered_map::end and dereferencing the result instead of (potentially expensively?) constructing std::out_of_range objects.

  • allbyregionactiveonly
  • allbyregionactivepreview
  • tasks/read_updates.cpp
  • HighwaySystem::stats_csv
  • ConnectedRoute::ConnectedRoute should stay as-is, because ErrorList.
  • Route::Route should stay as-is, because ErrorList.
  • Route::compute_stats_r: [] +=
  • HighwaySegment::compute_stats_t: [] +=
  • TravelerList::TravelerList (as of d56e5a6) can stay. A populated listupdates set with missing or malformed entries, missing info from git, should happen just about never in production. If it does, one lookup per traveler is inexpensive.

count

Consider using std::unordered_map::find and dereferencing the result instead of discarding it and searching again with std::unordered_map::at or whatever
All there is right now, at least in the master branch, is 9e2e4ef.

  • Save result of active_only_mileage_by_region.find(region) as an iterator
  • review how system_region_miles works (and all occurrences) -- does/can it just return 0?
  • first half unnecessary

all occurrences

[] may be accompanied by other unnecessary operations
.find cases can often have results saved in a variable
insert and emplace may benefit from better use of their respective construction methods; try piecewise-construct
...and whatever else

  • active_only_mileage_by_region
  • active_preview_mileage_by_region
  • alt_label_hash
  • alt_list_hash
  • code_hash
  • listupdates
  • mileage_by_region (and sysmbr in the r2 branch)
  • pri_label_hash
  • pri_list_hash
  • root_hash
  • system_region_mileages

Metadata

Metadata

Assignees

No one assigned

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions