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.

Sign up for GitHub

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

Jump to bottom

proposal: slices: add At function #65530

Closed
igoracmelo opened this issue Feb 5, 2024 · 9 comments
Closed

proposal: slices: add At function #65530

igoracmelo opened this issue Feb 5, 2024 · 9 comments
Labels
Proposal
Milestone
Proposal

Comments

@igoracmelo
Copy link

Proposal Details

I propose to add a function At in slices package, which would would return the value at a specific index of a slice, but with some caveats. See below

Function signature

// package slices

// `At` returns the element at index `i` and `true`.
// If `i` is negative, `At` counts backwords starting from the last item of the array.
// If the index is outside the bounds of the slice, `At` returns a zero value and `false`.
func At[T any](s []T, i int) (val T, ok bool)

Example

s := []string{"apple", "mango", "banana"}

slices.At(s, 0)  // "apple", true
slices.At(s, 1)  // "mango", true
slices.At(s, 2)  // "banana", true
slices.At(s, 3)  // "", false
slices.At(s, -1) // "banana", true

// This scenario can have two behaviors:
// Either return zero and false, or use index = index % len(s)
slices.At(s, -4) // "", false (this behavior can be discussed)

Why

1. slices.At would never panic

It is easy miss a bound check or to do it incorrectly

// will panic if i is >= len(s) or i is negative
val := s[i]

// will panic if len(s) = 0
val := s[len(s)-1] 

// safely get any item of the slice
val, ok := slices.At(s, i)

// safely get the last item of the slice
val, ok := slices.At(s, len(s)-1)
val, ok := slices.At(s, -1)

2. slices.At could receive negative indexes (Optional)

Similarly to how Python works, or how JavaScript Array.prototype.at method works, passing a negative index gets an element in a backward fashion.
This is useful for safely getting the last item of the array or penultimate.

I don't think this is critical to the proposal. If it is somewhat polemic, we can ignore this part and treat negative indexes as an error.

3. slices.At would make bound checks more explicit

Before:

func findFirstUser() (*User, error) {
  users, err := findUsers(...)
  if err != nil {
    return nil, err
  }
  return users[0], nil
}

After:

func findFirstUser() (*User, error) {
  users, err := findUsers(...)
  if err != nil {
    return nil, err
  }
  if user, ok := slices.At(users, 0); ok {
    // user can be safely accessed, returned, and so on
    return user, nil
  }
  return nil, errors.New("no user found")
}
@gopherbot gopherbot added this to the Proposal milestone Feb 5, 2024
@adonovan
Copy link
Member

adonovan commented Feb 5, 2024

Addressing the goals in order:

  1. never panicking: you still have to remember to use it. If you succeed at that, you'll equally succeed at checking your bounds and avoiding a panic the old fashioned way: if len(users) < 1 { return nil, fmt.Errorf("no users") }
  2. negative indices: this Python convention is appropriate for Python, where all indexing behaves consistently this way, but not for Go, where negative indices are consistently disallowed.
  3. explicitness: the old fashioned way makes the bounds check equally explicit.

I'm not convinced.

@igoracmelo
Copy link
Author

@adonovan

Thank you for your feedback.

  1. I would disagree. Off by one errors are pretty common, and can be missed at code reviews.
    You may remember you have to check the bounds and do it improperly.
  2. This is not a Go convention, but it could be the convention for this specific function. Similar to JavaScript Array.prototype.at. But this feature for me would be very optional.
  3. They are both explicit, but slices.At make it clear wether you accessed a valid index, without breaking the flow of application.

@igoracmelo
Copy link
Author

This could also be a comma syntax for slice[index], but I found that proposal (#54192) and it was not accepted.

@earthboundkid
Copy link
Contributor

See #53510 (comment).

@fzipp
Copy link
Contributor

fzipp commented Feb 6, 2024

  1. I would disagree. Off by one errors are pretty common, and can be missed at code reviews.
    You may remember you have to check the bounds and do it improperly.

This function makes it worse, because a -1 off-by-one error silently returns the incorrect (last) value instead of failing promptly.

@ianlancetaylor
Copy link
Contributor

This sounds like something that would be better implemented first outside of the standard library, and we can see whether people find it useful.

@igoracmelo
Copy link
Author

@fzipp that is why the idea of negative indexes is optional.
It could otherwise return zero, false

@rsc rsc changed the title proposal: slices: add At function proposal: slices: add At function Feb 8, 2024
@rsc
Copy link
Contributor

rsc commented Feb 9, 2024

Based on the discussion above, this proposal seems like a likely decline.
— rsc for the proposal review group

@rsc
Copy link
Contributor

rsc commented Feb 14, 2024

No change in consensus, so declined.
— rsc for the proposal review group

@rsc rsc closed this as completed Feb 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Proposal
Projects
Proposals
Status: Declined
Milestone
Proposal
Development

No branches or pull requests
7 participants
@rsc @earthboundkid @fzipp @ianlancetaylor @adonovan @gopherbot @igoracmelo