On 04/03/2012 07:35 PM, 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:
1. The clarity of the code of Data.ByteString.unsnoc would profit from using your 'unsafeInit and unsafeLast' functions. The same can be said about `uncons`. `unsnoc` is pretty much a product of copy-and-paste.
2. It might well be that some other places could profit from your 'unsafeInit' and 'unsafeLast' functions; e.g., the 'init' and 'last' functions. It would be great if you also updated other related definitions. Good point. I updated the patch.
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. Done. Thanks for the review!