PARTIAL 325 Fix - HashTableAnalysis C++ example - #1137
Open
GalinaP7 wants to merge 1 commit into
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fixes the C++ example and accompanying text in Section 2.8 (Hash Table Analysis). The example previously measured the wrong thing and didn't match the surrounding prose or the figure.
fixes #325
What's fixed
Documentation accuracy
std::vectorhas nocontainsmethod; the example usesstd::findfor the vector andfind/containsfor the hash table.Timing correctness
a(container size) to report the average time per single search operation, rather than the cumulative time of searching every element. This is what the "Time to Complete Contains Operation" framing in the text (and prior figure) actually describes, and it produces the correct O(n) / O(1) trend instead of an O(n²) cumulative total for the vector case.What's left to do
Figure (
fig-vectvshash-cpp) — not included in this PRvectvshash.pngreflects the old, incorrect cumulative-timing measurement (0–50 second scale) and no longer matches the corrected code's output (nanosecond/microsecond scale).<figure>block and removed its<xref>from the prose so the doc builds cleanly without a misleading image.vectvshash.png, after which the figure block can be uncommented and the xref restored.How this was verified
I compiled and ran the corrected code locally (g++,
-O2 -std=c++17) across the full range (10,000 to 990,000 elements, step 20,000) to confirm the fix actually produces the expected trend before writing up the results in the text.This is the run that replaced the old cumulative-timing approach (which summed the time to search every element rather than averaging per-operation time, and produced a misleading result that didn't match the O(n)/O(1) claims in the text).
Tested on local build
Reviewed by @harrisonj2-v