-
Notifications
You must be signed in to change notification settings - Fork 463
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good to me. good catch
if (i == 0) { | ||
final List<Integer> resultList = new ArrayList<>(resultLength * 2); | ||
int row = numRows - 1; | ||
int col = numCols - 1; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
love the name chanes
final int[] result = new int[resultSize]; | ||
for (int i = 0; i < resultSize; i++) { | ||
result[resultSize - 1 - i] = resultList.get(i); | ||
} | ||
return result; |
There was a problem hiding this comment.
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()
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Closes #46