2012/4/3 Duncan Coutts
On 3 April 2012 13:35, Simon Meier
wrote: Hi Mikhail,
I've never used these functions in my code. So I cannot judge their usefulness. Having inspected your patch, I see the following points for optimization:
3. The implementation of Data.ByteString.Lazy.unsnoc is too strict in my opinion. I would expect it to return 'Just <combined-init-and-last-computation>', as soon as it has proven that the ByteString is non-empty. To avoid unnecessary code duplication, it might make sense to inline only the wrapper and let GHC decide what it wants to do for the actual worker.
Actually, I think it doesn't make sense to provdide unsnoc for lazy bytestring at all. It's a bit of a silly operation for lazy bytestrings for the same reason as for lists. It's reasonable for strict bytestrings because you have the same access to the end of the string as the beginning, but the same is not true of lazy bytestrings.
I agree. I don't see a usecase for unsnoc on lazy bytestrings in production code. It nevertheless might make sense to provide it for one-off scripting code and for interface consistency. The necessarily bad implementation shows in the runtime given in the comment. Hence, I'd marginally vote for inclusion of the lazy bytestring unsnoc. @mikkhail: Note that at least the runtime for Data.ByteString.Lazy.Char8 is wrong. It should be O(n/c) instead of O(1). best regards, Simon