Skip to content

PARTIAL 325 Fix - HashTableAnalysis C++ example - #1137

Open
GalinaP7 wants to merge 1 commit into
pearcej:masterfrom
GalinaP7:2.8Listing6
Open

PARTIAL 325 Fix - HashTableAnalysis C++ example#1137
GalinaP7 wants to merge 1 commit into
pearcej:masterfrom
GalinaP7:2.8Listing6

Conversation

@GalinaP7

Copy link
Copy Markdown
Contributor

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

  • Clarified that std::vector has no contains method; the example uses std::find for the vector and find/contains for the hash table.
  • Reworded the "same operation" description to accurately reflect what the C++ and Python listings each do.
  • Removed the claim about searching for random numbers (the code searches sequentially for every value in the container), matching what's actually implemented.

Timing correctness

  • The build/fill step for both the vector and hash table is now excluded from the timed region (only the search loop is measured).
  • The total elapsed time for each container's full search loop is now divided by 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.
  • Confirmed via local runs (up to ~990,000 elements) that vector average search time grows linearly with size, and hash table average search time stays flat (~2 ns) regardless of size.

What's left to do

Figure (fig-vectvshash-cpp) — not included in this PR

  • The existing vectvshash.png reflects the old, incorrect cumulative-timing measurement (0–50 second scale) and no longer matches the corrected code's output (nanosecond/microsecond scale).
  • I've commented out the <figure> block and removed its <xref> from the prose so the doc builds cleanly without a misleading image.
  • A new chart needs to be generated from the corrected code's output (size vs. average vector/hash-table search time) and should replace vectvshash.png, after which the figure block can be uncommented and the xref restored.
  • I'm not sure how the original chart images in this book were generated — currently separatly working on trying to create a graph via Excel.

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.

image
  • The values filled into the results paragraph were calculated directly from this output, not estimated.

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

@GalinaP7 GalinaP7 changed the title Fix HashTableAnalysis C++ example PARTIAL Fix - HashTableAnalysis C++ example Aug 13, 2026
@GalinaP7 GalinaP7 changed the title PARTIAL Fix - HashTableAnalysis C++ example PARTIAL 325 Fix - HashTableAnalysis C++ example Aug 13, 2026

@pearcej pearcej left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thhis is clearly a really important change.

@wrigjl What are your thoughts?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Listing 6 in Section 2.8 does not test the contains operation

2 participants