Re: Cleaning up Differ

[Piotr Piastucki <leech miranda gmail com>]

A small correction - the code after removing the checks in 2) will be of 
course as follows:

  def _merge_blocks(self, using, low_seq, high_seq, last_diff):
       LO, HI = 1,2
       lowc  = using[low_seq][ 0][LO]
       highc = using[low_seq][-1][HI]
       lowc  =  min(lowc,  using[not low_seq][ 0][LO])
       highc =  max(highc, using[not low_seq][-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]

Cheers,
Piotr

Piotr Piastucki wrote:
> 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
> >