Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MapOf - Document zero value in the context of Load and whether or not it applies to deleted items during Range #119

Closed
flowchartsman opened this issue Feb 18, 2024 · 1 comment · Fixed by #121
Labels
documentation Improvements or additions to documentation question Further information is requested

Comments

@flowchartsman
Copy link

flowchartsman commented Feb 18, 2024

The godoc for MapOf.Load states the following (emphasis mine):

Load returns the value stored in the map for a key, or nil if no value is present. The ok result indicates whether value was found in the map.

This is probably a copy-and-paste from the non-generic map implementation, since it does not make sense in the context of a generic data structure when the return type is not a pointer. To be more concrete, there cannot be a nil return from MapOf[string, string].Load(), so it should probably read something like:

Load returns a value of type V along with an boolean indicating whether the key was present in the map. If the key was not present, value will be the zero value of type V.

This brings up a related question about MapOf.Range. The godoc there says (emphasis mine):

Range does not necessarily correspond to any consistent snapshot of the Map's contents: no key will be visited more than once, but if the value for any key is stored or deleted concurrently, Range may reflect any mapping for that key from any point during the Range call.

Here "any mapping" makes sense in the storage context --if one or more Ps stores a value while we are iterating, we might see any one of them or none at all-- but it is unclear what "any mapping" means in the case of deletion. What happens if an entry is deleted before iteration reaches it? Are we guaranteed to never see it, or could we get the zero value instead? The docs here should be specific, since range does not give us any indication if the value we're seeing might have been deleted. This means that, if it is important to us not to use a deleted value in Range, we need to synthesize a way to do it ourselves using the zero value of the type. In the case of primitive types, this would mean never storing the zero value directly, while in the case of structs, it would mean ensuring that at least one field is never stored with a zero value, so that we can can use it to check if the entry was deleted.

I'll also note that the Range docs say that it's safe to modify the map during range; does this apply to deletion?
Edit: I can answer my own last question here; this test seems to spell out pretty clearly that delete while ranging is fine.

@flowchartsman flowchartsman changed the title MapOf Document zero value, and clarify Range documentation on delete. MapOf Document zero value in the context of Load and whether or not it applies to deleted items during Range Feb 18, 2024
@flowchartsman flowchartsman changed the title MapOf Document zero value in the context of Load and whether or not it applies to deleted items during Range MapOf - Document zero value in the context of Load and whether or not it applies to deleted items during Range Feb 18, 2024
@puzpuzpuz
Copy link
Owner

Thanks for the feedback. I'll update the godoc.

Edit: I can answer my own last question here; this test seems to spell out pretty clearly that delete while ranging is fine.

Yes, map modification while iterating over it is supported.

@puzpuzpuz puzpuzpuz added documentation Improvements or additions to documentation question Further information is requested labels Feb 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation question Further information is requested
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants