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

[levenshtein][bug] Fix bug when # of changes exceeds max length of text #47

Merged
merged 1 commit into from
Apr 16, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions ticker-sample/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ apply plugin: 'com.android.application'

android {
compileSdkVersion 25
buildToolsVersion "25.0.0"
buildToolsVersion "25.0.2"

defaultConfig {
applicationId "com.robinhood.ticker.sample"
Expand All @@ -15,8 +15,8 @@ dependencies {
compile fileTree(dir: 'libs', include: ['*.jar'])
compile project(':ticker')

compile 'com.android.support:appcompat-v7:25.0.0'
compile 'com.android.support:recyclerview-v7:25.0.0'
compile 'com.android.support:appcompat-v7:25.3.1'
compile 'com.android.support:recyclerview-v7:25.3.1'

compile 'io.reactivex:rxjava:1.1.1'
compile 'io.reactivex:rxandroid:1.1.0'
Expand Down
2 changes: 1 addition & 1 deletion ticker/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ apply plugin: 'com.android.library'

android {
compileSdkVersion 25
buildToolsVersion "25.0.0"
buildToolsVersion "25.0.2"

resourcePrefix 'ticker_'

Expand Down
57 changes: 31 additions & 26 deletions ticker/src/main/java/com/robinhood/ticker/LevenshteinUtils.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
package com.robinhood.ticker;

import java.util.ArrayList;
import java.util.List;

/**
* Helper class to compute the Levenshtein distance between two strings.
* https://en.wikipedia.org/wiki/Levenshtein_distance
Expand Down Expand Up @@ -27,11 +30,9 @@ public static int[] computeColumnActions(char[] source, char[] target) {
final int targetLength = target.length;
final int resultLength = Math.max(sourceLength, targetLength);

final int[] result = new int[resultLength];

if (sourceLength == targetLength) {
// No modifications needed if the length of the strings are the same
return result;
return new int[resultLength];
}

final int numRows = sourceLength + 1;
Expand Down Expand Up @@ -60,38 +61,42 @@ public static int[] computeColumnActions(char[] source, char[] target) {
}

// Reverse trace the matrix to compute the necessary actions
int i = numRows - 1;
int j = numCols - 1;
int resultIndex = resultLength - 1;
while (resultIndex >= 0) {
if (i == 0) {
final List<Integer> resultList = new ArrayList<>(resultLength * 2);
int row = numRows - 1;
int col = numCols - 1;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

love the name chanes

while (row > 0 || col > 0) {
if (row == 0) {
// At the top row, can only move left, meaning insert column
result[resultIndex] = ACTION_INSERT;
j--;
} else if (j == 0) {
resultList.add(ACTION_INSERT);
col--;
} else if (col == 0) {
// At the left column, can only move up, meaning delete column
result[resultIndex] = ACTION_DELETE;
i--;
resultList.add(ACTION_DELETE);
row--;
} else {
final int top = matrix[i-1][j];
final int left = matrix[i][j-1];
final int topLeft = matrix[i-1][j-1];
final int insert = matrix[row][col-1];
final int delete = matrix[row-1][col];
final int replace = matrix[row-1][col-1];

if (topLeft <= top && topLeft <= left) {
result[resultIndex] = ACTION_SAME;
i--;
j--;
} else if (top <= left) {
result[resultIndex] = ACTION_DELETE;
i--;
if (insert < delete && insert < replace) {
resultList.add(ACTION_INSERT);
col--;
} else if (delete < replace) {
resultList.add(ACTION_DELETE);
row--;
} else {
result[resultIndex] = ACTION_INSERT;
j--;
resultList.add(ACTION_SAME);
row--;
col--;
}
}
resultIndex--;
}

final int resultSize = resultList.size();
final int[] result = new int[resultSize];
for (int i = 0; i < resultSize; i++) {
result[resultSize - 1 - i] = resultList.get(i);
}
return result;
Copy link

@danh32 danh32 Apr 16, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would it be painful to make this method's return type List<Int>? would avoid needing this conversion loop here and the quickreturn above could return Collections.emptyList()

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah good point i'll just return them in list

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

uh hm just realized that the above can't return empty list because it needs to return the number of columns with no modifications, and this iteration also adds the updates in reverse since we do reverse trace of the matrix to get the modifications. eh i'll just leave it for now lol.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,22 @@ public void test_completelyDifferent() {

@Test
public void test_shift() {
// The reason why this isn't 20001 which is a shift of "234" over is because that
// would require 5 changes rather than 4.
runTest("1234", "2345", "0000");
}

@Test
public void test_mix1() {
// "15" should shift to the right 1 place, then delete two columns after "15"
runTest("15233", "9151", "100220");
}

@Test
public void test_mix2() {
runTest("12345", "230", "20020");
}

@Test
public void test_currency1() {
runTest("$123.99", "$1223.98", "00010000");
Expand Down