refactor(city): inject CSV data into get_city and iter_cities (#106)#112
Open
costajohnt wants to merge 2 commits into
Open
refactor(city): inject CSV data into get_city and iter_cities (#106)#112costajohnt wants to merge 2 commits into
costajohnt wants to merge 2 commits into
Conversation
…e#106) Both functions now take a data buffer and length instead of reading from the cities.h globals. main.c passes cities, cities_len so end-user behavior is unchanged. The payoff is in tests. A new test_get_city_duplicate_names feeds a two-row fixture with the same name in both possible orderings and verifies the higher-population row always wins, which is the case called out in the issue after PR da-luce#79. Closes da-luce#106
Reflow iter_cities signature and the test CSV string-literal initializers so clang-format --check passes under the pinned CI version.
Codecov Report❌ Patch coverage is
🚀 New features to boost your workflow:
|
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
1 participant
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.
Summary
Un-hardcodes
get_cityanditer_citiesper issue #106. Both functions now take a CSV byte buffer and its length as explicit parameters instead of reading thecities.hglobals directly.main.cpassescities, cities_lento both call sites, so end-user behavior is unchanged.test/city_test.cupdates to pass the same arguments for all existing tests, confirming the default path still works.Why
The issue calls out that duplicate-name handling is hard to test when the function only reads from a compiled-in global. The only way to stress it is via the full
cities.csvand we can't easily guarantee both orderings of a given name are present.The new test
test_get_city_duplicate_namesfeeds two tiny two-row CSV fixtures with aLondonin each ordering:and asserts the ~8.9M row wins either way. This would have caught the binary-search regression fixed in PR #79 directly.
Also added
test_iter_cities_null_data_should_not_crashsince the new parameter adds a new null-handling path.Test plan
meson test -C build city_test— 6/6 pass (was 4/4 before, +2 new)meson test -C build— 8/8 suites pass./build/astroterm --help— binary still runscities.csvorcities.hgenerationCloses #106