Re: Cleaning up Differ

[Piotr Piastucki <leech miranda gmail com>]

Hi,

I am a bit surprised that the patch is not self-explanatory, because the 
code in diffutil is quite simple after all, but here is the info you 
requested:

1) block is not need if
len(using[0])==0 OR len(using[1])==0
because of:
...
           block = self._merge_blocks( using, base_seq, high_seq, block)

           if len(using[0])==0:
               assert len(using[1])==1
               yield None, using[1][0]
           elif len(using[1])==0:
               assert len(using[0])==1
               yield using[0][0], None
           else:
...
so moving the call here does not change anything:
...
               yield using[0][0], None
           else:
               block = self._merge_blocks( using, base_seq, high_seq, 
block)
...


2) Hence self._merge_blocks is now called if and only if both 
len(using[0]) and len(using[1]) are != 0, then the following checks in 
merge_blocks() become redundant:
a)        if len(using[not low_seq]):
b)        if len(using[i]):

As a result merge_blocks() may be simplified to:

   def _merge_blocks(self, using, low_seq, high_seq, last_diff):
        LO, HI = 1,2
        lowc  =  min(using[low_seq][ 0][LO],  using[not low_seq][ 0][LO])
        highc =  max(using[low_seq][-1][HI], using[not low_seq][-1][HI])
        lowc  =  min(using[0][ 0][LO], using[1][ 0][LO])
        highc =  max(using[0][-1][HI], using[1][-1][HI])
        low = []
        high = []
        for i in (0,1):
           if len(using[i]):
               d = using[i][0]
               low.append(  lowc  - d[LO] + d[2+LO] )
               d = using[i][-1]
               high.append( highc - d[HI] + d[2+HI] )
        return low[0], high[0], lowc, highc, low[1], high[1]


3) Finally, optimizing min/max into a single line is pretty 
straight-forward.

Should you have any questions please let me know.

Thanks,
Piotr



Kai Willadsen wrote:
> 2009/6/7 Piotr Piastucki <leech miranda gmail com>:
>   
> > Hi,
> >
> > I am attaching yet another patch that simplifies Differ class.
> >     
>
> I had a look at this patch (this is from bug 577092, right?)
> yesterday. In quick testing, it seems like the new code has equivalent
> effect, but I don't think that this is obvious from the patch.
>
> For example, I couldn't convince myself that moving the call to
> self._merge_blocks() didn't change behaviour. Some explanation of the
> logic behind the change would really help here.
>
> Kai
>