Skip to content

Conversation

@wvpm
Copy link
Contributor

@wvpm wvpm commented Nov 14, 2025

When the economy is split from CountryInstance (see #638 ) we may not have access to the CountryInstance when placing orders and can use the country index instead. There market does nothing with CountryInstance. It merely accesses IndexedFlatMaps with CountryInstance as key.

Depends on #658 + #659

@wvpm wvpm added enhancement New feature or request topic:codestyle labels Nov 14, 2025
@wvpm wvpm force-pushed the market_use_country_index_instead_of_instance branch from 616cc64 to 482bad4 Compare November 14, 2025 10:44
@schombert
Copy link

Once you start passing around size_t indexes instead of types, you lose a lot of type safety because it is easy to accidentally pass an index to a different array. The solution to this problem is to introduce strongly typed index wrappers around the size_t values so that you can use the type system to ensure that array lookups and function calls can only be performed with an index of the right kind.

@wvpm
Copy link
Contributor Author

wvpm commented Nov 15, 2025

Once you start passing around size_t indexes instead of types, you lose a lot of type safety because it is easy to accidentally pass an index to a different array. The solution to this problem is to introduce strongly typed index wrappers around the size_t values so that you can use the type system to ensure that array lookups and function calls can only be performed with an index of the right kind.

True. We consider using https://github.com/foonathan/type_safe to tag types.
That is for a follow-up PR though.

@wvpm wvpm force-pushed the market_use_country_index_instead_of_instance branch 3 times, most recently from dd82717 to 92070db Compare November 19, 2025 11:17
@wvpm wvpm enabled auto-merge November 19, 2025 11:24
@wvpm wvpm force-pushed the market_use_country_index_instead_of_instance branch 5 times, most recently from ae05d99 to 08cb395 Compare November 20, 2025 09:25
@wvpm wvpm mentioned this pull request Nov 20, 2025
@wvpm wvpm force-pushed the market_use_country_index_instead_of_instance branch 6 times, most recently from 9074bfd to 4874d86 Compare November 21, 2025 19:50
@wvpm wvpm force-pushed the market_use_country_index_instead_of_instance branch from 4874d86 to 1df5d14 Compare November 21, 2025 22:23
@wvpm wvpm merged commit de52d16 into master Nov 22, 2025
16 checks passed
@Spartan322 Spartan322 deleted the market_use_country_index_instead_of_instance branch November 22, 2025 23:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants