| Message: 24175 |
 |
BY: Rob Morris (irongaze)
DATE: 2007-06-24 06:20
SUBJECT: RE: Diff::LCS bug - spurious remove action OK, so the cause is the way traverse_sequences handles String (and ONLY string!) arguments. Seems the author wanted to have the elements returned be strings, rather than character fixnums, ie "a" vs 97.
As a result, the code sets a flag:
string = seq1.kind_of?(String)
If string is true, instead of using seq1[index] to access each element, it uses seq1[index,1].
This works, unless index = seq1.length, in which case you get an empty string "" instead of nil, which you might expect. The code, naturally, tests with .nil? on each element. And generates the incorrect remove action.
You can fix this one of two ways. If (like me) you don't want strings for your sub-elements, just set string = false on line 399 of lcs.rb. This should result in a major speedup as well, as it prevents the generation of a zillion one character strings that you'll most likely be concatenating anyway. (you can then remove all the conditionals on the 'string' switch too, of course...)
The other option is to go fix the .nil? calls to be .empty? instead, and (if you're not on Rails) monkey patch the Nil object to respond true to empty? calls. Fun stuff. :-)
Hope this helps someone out there... | |
Start New Thread
Diff::LCS bug - spurious remove action