3 ms·

The irony in your smug reply is, it echoes the broken leftpad mentality of javascript programmers. Go is statically typed, and this either requires generics or

by tagrun 10y ago

The irony in your smug reply is, it echoes the broken leftpad mentality of javascript programmers. Go is statically typed, and this either requires generics or a new built-in for just reversing an array/slice. And I can't see how a trivial operation as array reversion is worth it, and there's just no end to adding such trivial operations. If you need a slice with reverse, just add a typedef and define reverse on it --it's just a trivial loop. What is it that you want to say? That array reversal too much for mortals and better be left to array experts? Array sorting is both important and nontrivial subject, and its place in the library is justified (similar to the situation with C). BTW, I don't understand why your for-loop runs backwards (which makes the code look awkward to me) or why you have to define opp in the loop body. Just define len(a)-1 as a variable in the initialization and be done with it.

oconnor663 10y ago
Guilty as charged on the smugness, sorry about that. This reply is meant to be smugness-free. My example above is the semi-official one from https://github.com/golang/go/wiki/SliceTricks https://github.com/golang/go/wiki/SliceTricks. The `oop` expression contains a reference to i, and so it needs to be in the loop body. I'm not sure why they opted to do it in reverse though. (Maybe they don't want to compute `len(a)/2` every time, and don't want to use a temp variable? Not sure if it would get optimized away.) In my head there are three reasons (apart from my own laziness) that I like having reverse() in a language's standard library: - Code is read more often than it's written. A for loop like this takes time to read, if you don't already know what it does. - Tricky loops hide bugs. If we forgot the `-1` term in the initializer, I'd probably miss it in a code review, and the bug would only show up for even-length lists. - Expert attention turns out to be useful here! In languages that check array bounds by default, you can gain a little speed by avoiding bounds checks in reverse(). Rust does this (https://doc.rust-lang.org/src/core/up/src/libcore/slice.rs.html#442 https://doc.rust-lang.org/src/core/up/src/libcore/slice.rs.h...), but I wouldn't want an `unsafe` block like that in code I had to copy-paste around.
- tagrun 10y ago
  Yes, you can miss anything in any piece of code in a review. No, this doesn't mean you should use someone else's code for trivial stuff a la leftpad. However, if you're saying that you don't have confidence that you can properly review an array reversal, then it's something personal and you're extrapolating, and maybe shouldn't do code reviews at all (as harsh as this may sound, it's not meant to be a shallow insult ---take a second an think about it in the context you've given: if you're going to miss that -1 or the fact that array indices start from 0 rather than 1 in Go, how can you expect anyone to trust you with reviewing their code that involves arrays?). Using 3rd party code for "basic" stuff only makes sense to me when it's a tricky algorithm to get right (such as quicksort) and the author of the 3rd part code is a renowned/reputable top developer. I definitely don't trust the b+-tree implementation of some random dude on github(which what javascript people do mostly). No idea who wrote that piece of code, but you can simply define it without the -i part, and write a[last-i] = a[last-i], a[i] in the loop (and better not call it "oop" ---whatever that means). That way, it's very readable and understandable to me: for i, last := 0, len(a)-1; i < len(a)/2; i++ { a[i], a[last-i] = a[last-i], a[i] } I still don't see anything in what you wrote that justifies adding a new built-in function for a trivial operation such as reverse, or generics to the language as a whole. If profiling shows that bounds checking in reverse() is a bottleneck in your program (although it's very difficult for me to imagine that this will happen a real-world program), then you probably don't want to use that unsafe code went through "expert attention". While being a mortal without any specific degree on array handling myself, in the unlikely event that I find reverse() to be a bottleneck in my program, I'd rather write an assembly function which makes use of SSE2/AVX2 to get real speed-up (note that LLVM can't autovectorize that Rust code for you, that code needs a rewrite it you want vectorization).

by tagrun 10y ago

oconnor663 10y ago

Guilty as charged on the smugness, sorry about that. This reply is meant to be smugness-free. My example above is the semi-official one from https://github.com/golang/go/wiki/SliceTricks https://github.com/golang/go/wiki/SliceTricks. The `oop` expression contains a reference to i, and so it needs to be in the loop body. I'm not sure why they opted to do it in reverse though. (Maybe they don't want to compute `len(a)/2` every time, and don't want to use a temp variable? Not sure if it would get optimized away.) In my head there are three reasons (apart from my own laziness) that I like having reverse() in a language's standard library: - Code is read more often than it's written. A for loop like this takes time to read, if you don't already know what it does. - Tricky loops hide bugs. If we forgot the `-1` term in the initializer, I'd probably miss it in a code review, and the bug would only show up for even-length lists. - Expert attention turns out to be useful here! In languages that check array bounds by default, you can gain a little speed by avoiding bounds checks in reverse(). Rust does this (https://doc.rust-lang.org/src/core/up/src/libcore/slice.rs.html#442 https://doc.rust-lang.org/src/core/up/src/libcore/slice.rs.h...), but I wouldn't want an `unsafe` block like that in code I had to copy-paste around.

tagrun 10y ago
Yes, you can miss anything in any piece of code in a review. No, this doesn't mean you should use someone else's code for trivial stuff a la leftpad. However, if you're saying that you don't have confidence that you can properly review an array reversal, then it's something personal and you're extrapolating, and maybe shouldn't do code reviews at all (as harsh as this may sound, it's not meant to be a shallow insult ---take a second an think about it in the context you've given: if you're going to miss that -1 or the fact that array indices start from 0 rather than 1 in Go, how can you expect anyone to trust you with reviewing their code that involves arrays?). Using 3rd party code for "basic" stuff only makes sense to me when it's a tricky algorithm to get right (such as quicksort) and the author of the 3rd part code is a renowned/reputable top developer. I definitely don't trust the b+-tree implementation of some random dude on github(which what javascript people do mostly). No idea who wrote that piece of code, but you can simply define it without the -i part, and write a[last-i] = a[last-i], a[i] in the loop (and better not call it "oop" ---whatever that means). That way, it's very readable and understandable to me: for i, last := 0, len(a)-1; i < len(a)/2; i++ { a[i], a[last-i] = a[last-i], a[i] } I still don't see anything in what you wrote that justifies adding a new built-in function for a trivial operation such as reverse, or generics to the language as a whole. If profiling shows that bounds checking in reverse() is a bottleneck in your program (although it's very difficult for me to imagine that this will happen a real-world program), then you probably don't want to use that unsafe code went through "expert attention". While being a mortal without any specific degree on array handling myself, in the unlikely event that I find reverse() to be a bottleneck in my program, I'd rather write an assembly function which makes use of SSE2/AVX2 to get real speed-up (note that LLVM can't autovectorize that Rust code for you, that code needs a rewrite it you want vectorization).