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

slices: more explicit doc for Delete, DeleteFunc, Replace #64329

Open
Deleplace opened this issue Nov 22, 2023 · 4 comments
Open

slices: more explicit doc for Delete, DeleteFunc, Replace #64329

Deleplace opened this issue Nov 22, 2023 · 4 comments
Labels
Documentation NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@Deleplace
Copy link
Contributor

This a minor documentation proposal.

I've witnessed many variations of these two incorrect idioms:

    slices.Delete(s, i, i+1)
    u := slices.Delete(t, i, i+1)
    // keep using t and u...
  • In the first one, the caller is ignoring the returned value, because they assume Delete works in-place.
  • In the second one, the caller is assigning the returned value to a different variable, because they assume Delete creates a new slice.

Delete does operate in-place, but because of the nature of slice headers, it also returns a new slice header. This API is similar to append.

The documentation for functions Delete, DeleteFunc, Replace are already correct. But depending on how the user understands the words "remove" and "modified", they may still end up using one of the incorrect idioms, relying on the wrong assumption that the API must be either 100% in-place or 100% by copy.

Here is (in bold) my suggested addition to the docs:

func Delete

Delete removes the elements s[i:j] from s, returning the modified slice.
Delete operates in place, which is destructive for the contents of the original slice.
Delete panics if j > len(s) or s[i:j] is not a valid slice of s.
Delete is O(len(s)-i), so if many items must be deleted, it is better to
make a single call deleting them all together than to delete one at a time.
Delete zeroes the elements s[len(s)-(j-i):len(s)].

func DeleteFunc

DeleteFunc removes any elements from s for which del returns true, returning the modified slice.
DeleteFunc operates in place, which is destructive for the contents of the original slice.
DeleteFunc zeroes the elements between the new length and the original length.

func Replace

Replace replaces the elements s[i:j] by the given v, and returns the
modified slice.
Replace operates in place, which is destructive for the contents of the original slice.
Replace panics if j > len(s) or s[i:j] is not a valid slice of s.
When len(v) < (j-i), Replace zeroes the elements between the new length and the original length.

@mknyszek mknyszek added this to the Backlog milestone Nov 22, 2023
@mknyszek mknyszek added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Nov 22, 2023
@mknyszek
Copy link
Contributor

@eliben
Copy link
Member

eliben commented Nov 22, 2023

Sounds like a good idea in general. Examples can also help and they show up in the docs in the right place

@Deleplace
Copy link
Contributor Author

@eliben examples are a great thing in general but the 3 funcs Delete, DeleteFunc, Replace already have an example showing the correct usage

@eliben
Copy link
Member

eliben commented Nov 22, 2023

@eliben examples are a great thing in general but the 3 funcs Delete, DeleteFunc, Replace already have an example showing the correct usage

There can be multiple examples per symbol if needed :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

3 participants