[darcs-users] darcs patch: rewrite partitionFL and partitionRL to reduce the numb...

[David Roundy]

On Thu, Oct 30, 2008 at 05:58:52AM +0000, Ganesh Sittampalam wrote:
> On Wed, 29 Oct 2008, Jason Dagit wrote:
> 
> >On Wed, Oct 29, 2008 at 1:35 PM, Ganesh Sittampalam <ganesh at earth.li> 
> >wrote:
> >>Hi David,
> >>
> >>No rush to deal with this one, I'm just sending it in case you want to
> >>and so that others can review it.
> >>
> >>This addresses a different quadratic blowup I found when running
> >>whatsnew -sl on a directory with lots of files in it. It's a nice
> >>advert for type witnesses being very helpful in creating confidence
> >>in a refactoring.
> >
> >Yes, the GADT stuff makes refactoring quite nice.
> >
> >I looked at this patch bundle, I don't really see any problems with
> >it, but I didn't look that closely.  Do you think it would be hard to
> >write QuickCheck properties that compare the new implementation with
> >the old one?  My reasoning is this: the old implementation was simpler
> >and easier to inspect, so we can use it as a test answer oracle with
> >reasonable confidence.
> 
> Yes, I think it would be quite easy to write properties - the tricky bit 
> is generating the selection function. I don't think the old implementation 
> was actually simpler though, as it relied on commuteWhatWeCan which isn't 
> really very simply.
> 
> For what it's worth I'm reasonably confident that I've reproduced the old 
> behaviour, though obviously QuickCheck properties wouldn't hurt. I think 
> my preferred approach to doing that would be to write them for the 
> specification I claim in the comments, and then to check both old and new 
> against the properties.

It looks good to me, and is going in.  I agree with your suggestion of how
to implement tests.  Can't coarbitrary be used to generate the selection
function?
-- 
David Roundy
http://www.darcs.net