fix "out of bounds"

[archimag]

Fix for https://github.com/froydnj/diff/issues/1

[froydnj]

I think this is mostly OK; I don't remember a lot of the details of the algorithm, though. Comments coming inline on the diff itself.

[froydnj]

This declaration should be removed.

[froydnj]

As I was commenting, I took a closer look at the code...does all this really stem from the initial creation of FP sharing a snake between all elements of FP? If so, wouldn't it be easier to simply initialize FP differently, by dropping the :INITIAL-ELEMENT and doing:

(loop for i from 0 below (length fp)
do (setf (aref fp i) (make-instance 'snake ...)))

? Like I said, I don't remember all the details of the algorithm, but looking at the code, my use of :INITIAL-ELEMENT there is very suspect.

[archimag]

Well, I rewrote the code.

Unfortunately, I'm not familiar with this algorithm and has not had the time
for understand. But I found the code in other language (C)​​ for compare and found
no significant difference with you code. So I made ​​a change that just work.

I use a diff for: http://cliki2.lisper.ru/history/RESTAS/3470385331-3488861177,
but encountered an error: http://cliki2.lisper.ru/history/RESTAS/3470385331-3511295325.
With my patch everything works fine.

P.S. I apologize for my terrible english.

[archimag]

I has united my changes on this issue in one commit

[archimag]

I sync this branch with last changes.

[archimag]

Hi. I would not want to seem intrusive, but is now important for me to get result on this issue. You could not dedicate this issue a little bit of his time? :)

[froydnj]

Hi, sorry for not responding. I'm not completely convinced that this can't be solved by simply initializing all the members of the FP array with their own instance of a SNAKE, rather than sharing one between them all. That should be an equivalent solution to what you're doing, but be somewhat clearer. Could you try that solution instead?

[archimag]

Oh, I looked at solutions in other languages ​​and found that this bug can be corrected very simple manner.

[froydnj]

OK, so I agree that fix is quite simple. :) But why does that fix things? Why 3 and not 4, or 2?

[archimag]

Yes, I know it does not very impressive, but I found 3 in the Python implementation and test it on their data. 1 or 2 is not enough if the original and modified texts do not have common parts. 3 was sufficient in all cases.

[froydnj]

Do you have a reference for the Python implementation? I know Python's standard library uses a different algorithm.

Honestly, the code was copied from other implementations originally, so I'm not opposed to a bit more copying. I'd just like to have a shot at understanding why we're copying this bit.

[archimag]

https://github.com/cubicdaiya/onp/blob/master/python/onp.py

In addition, this project have the implementation of this algorithm and the other languages.

[froydnj]

Well, they have a lot of helpful comments, don't they? Pulling; thanks for the fix.