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

sort: change the main example to use Slice instead of Sort #21989

Closed
agnivade opened this issue Sep 23, 2017 · 6 comments
Closed

sort: change the main example to use Slice instead of Sort #21989

agnivade opened this issue Sep 23, 2017 · 6 comments

Comments

@agnivade
Copy link
Contributor

@agnivade agnivade commented Sep 23, 2017

Hi,

The topmost example that we see if we open https://golang.org/pkg/sort/ uses the sort.Sort function. The use case it solves is the most common one which compares an attribute inside a struct.

I think its better to point people to the more comfortable and simple function sort.Slice for the most common case.

So, that would change -

// ByAge implements sort.Interface for []Person based on
// the Age field.
type ByAge []Person

func (a ByAge) Len() int           { return len(a) }
func (a ByAge) Swap(i, j int)      { a[i], a[j] = a[j], a[i] }
func (a ByAge) Less(i, j int) bool { return a[i].Age < a[j].Age }

func main() {
	people := []Person{
		{"Bob", 31},
		{"John", 42},
		{"Michael", 17},
		{"Jenny", 26},
	}

	fmt.Println(people)
	sort.Sort(ByAge(people))
	fmt.Println(people)

}

to

func main() {
	people := []Person{
		{"Bob", 31},
		{"John", 42},
		{"Michael", 17},
		{"Jenny", 26},
	}

	fmt.Println(people)
	sort.Slice(people, func(i, j int) bool {
		return people[i].Age < people[j].Age
	})
	fmt.Println(people)
}

I believe this would lead newcomers who don't know about the new sort.Slice function and have just opened the documentation page for the first time, to use a better and simpler option.

Would be happy to send a CL if you guys agree to this.

@mvdan
Copy link
Member

@mvdan mvdan commented Sep 23, 2017

This sounds like a good idea. But perhaps we should add a sort.Sort example then, showing a common case where sort.Slice isn't enough.

@agnivade
Copy link
Contributor Author

@agnivade agnivade commented Sep 23, 2017

Yep, there are 4 examples in the top section. All of them use sort.Sort. So, we would be just changing the most common use-case to sort.Slice. The remaining 3 will still use sort.Sort.

This was partly inspired by your CL which replaced lot of sort.Sort usages with sort.Slice. 😄 I just happened to look up the definition of sort.Slice and this idea hit me.

@mvdan
Copy link
Member

@mvdan mvdan commented Sep 23, 2017

I see - it sounds like the package examples should mention sort.Slice, yes. Feel free to send a CL. I don't guarantee it will be merged, and I won't approve it since I'm not that familiar with the package, but it seems like a good idea to me.

@mvdan mvdan added the Documentation label Sep 23, 2017
@mvdan mvdan changed the title sort: change the main example to use sort.Slice instead of sort.Sort sort: change the main example to use Slice instead of Sort Sep 23, 2017
@kennygrant
Copy link
Contributor

@kennygrant kennygrant commented Sep 23, 2017

There are already sort.Slice and sort.SliceStable example, they just aren't featured in the top 4 examples:

https://golang.org/pkg/sort/#Slice
https://golang.org/pkg/sort/#SliceStable

I agree they should have more prominence.

@agnivade
Copy link
Contributor Author

@agnivade agnivade commented Sep 23, 2017

Exactly. This is about getting sort.Slice some more love.

@gopherbot
Copy link

@gopherbot gopherbot commented Sep 23, 2017

Change https://golang.org/cl/65710 mentions this issue: sort: change main example to use Slice instead of Sort

@gopherbot gopherbot closed this in ff3123d Sep 24, 2017
@golang golang locked and limited conversation to collaborators Sep 24, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.