code review 3458041: io/ioutil: give ReadDir a sorting function

io/ioutil: give ReadDir a sorting function

[ality, 2010-12-04 01:36:24]

Hello rsc, r (cc: golang-dev@googlegroups.com),

I'd like you to review this change.

[r, 2010-12-04 02:52:14]

R=gri

[rsc1, 2010-12-06 19:37:20]

I am not sure this change is worthwhile.
Sorting by name is almost always what is wanted,
and when not, one can already call the general
sort appropriately (using os.Readdir if necessary
to avoid the cost of the first sort).

ioutil.ReadDir is a convenience wrapper;
as more arguments get added, especially
ones that almost never change, they lose
their convenience and their rationale.

[gri, 2010-12-07 00:07:42]

I agree with Russ; though I can see that on occasion one may want to sort
according to time stamps, and perhaps other things. It's informative though
that all uses in the golang.org source base only sort by name

I think this CL could be more interesting by changing the API a bit: Instead
of providing a sort mode, provide a sort function

func(x, y *os.FileInfo) bool

to ReadDir. If nil is provided, no sorting is happening. There should
probably be also a default comparison function for sort by name since it is
the common case.

- gri



On Fri, Dec 3, 2010 at 5:36 PM, <ality@pbrane.org> wrote:

> Reviewers: rsc, r,
>
> Message:
> Hello rsc, r (cc: golang-dev@googlegroups.com),
>
> I'd like you to review this change.
>
>
> Description:
> io/ioutil: configurable sort mode in ReadDir
>
> Please review this at http://codereview.appspot.com/3458041/
>
> Affected files:
>  M src/cmd/godoc/codewalk.go
>  M src/cmd/godoc/dirtrees.go
>  M src/cmd/godoc/godoc.go
>  M src/cmd/godoc/index.go
>  M src/pkg/io/ioutil/ioutil.go
>  M src/pkg/io/ioutil/ioutil_test.go
>  M src/pkg/path/path.go
>
>
>

[ality, 2010-12-15 08:45:26]

Hello gri (cc: golang-dev@googlegroups.com),

Please take another look.

[rsc1, 2010-12-15 14:13:50]

I'm still unhappy with adding arguments to ReadDir.
In the very few cases where you need to sort differently,
why not just let the caller re-sort?

Russ

[ality, 2010-12-15 18:02:41]

rsc@google.com once said:
> I'm still unhappy with adding arguments to ReadDir.
> In the very few cases where you need to sort differently,
> why not just let the caller re-sort?
> 
> Russ

I sent this just in case gri wanted to see the change
he requested. I'll abandon it now.

Thanks,
  Anthony

[ality, 2010-12-15 18:08:01]

*** Abandoned ***