Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update the levenshtein distance method in guides. #15955

Merged
merged 1 commit into from
Jun 28, 2014
Merged

Update the levenshtein distance method in guides. #15955

merged 1 commit into from
Jun 28, 2014

Conversation

JuanitoFatas
Copy link
Contributor

Copied from https://github.com/rails/rails/blob/master/railties/lib/rails/generators.rb#L258-L290.

Why I want to update? I update this because the current implementation has wrong result, if you copy this code into console:

def distance(s1, s2)
  s = s1.unpack('U*')
  t = s2.unpack('U*')
  m = s.length
  n = t.length

  # matrix initialization
  d = []
  0.upto(m) { |i| d << [i] }
  0.upto(n) { |j| d[0][j] = j }

  # distance computation
  1.upto(m) do |i|
    1.upto(n) do |j|
      cost = s[i] == t[j] ? 0 : 1
      d[i][j] = [
        d[i-1][j] + 1,      # deletion
        d[i][j-1] + 1,      # insertion
        d[i-1][j-1] + cost, # substitution
      ].min
    end
  end

  # all done
  return d[m][n]
end

And type distance 'kitten', 'sitting', the result is 2, which is wrong. It should be 3.

This kitten and sitting example is from wikipedia.

@JuanitoFatas JuanitoFatas changed the title Update the levenshtein distance method. Update the levenshtein distance method in guides. Jun 28, 2014
guilleiguaran added a commit that referenced this pull request Jun 28, 2014
Update the levenshtein distance method in guides.
@guilleiguaran guilleiguaran merged commit cad584e into rails:master Jun 28, 2014
@guilleiguaran
Copy link
Member

Did you tested generating guides after of this change?

@JuanitoFatas
Copy link
Contributor Author

@guilleiguaran Yes! I tested.

@JuanitoFatas JuanitoFatas deleted the levenshtein-guide branch June 28, 2014 15:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants