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
get longest palindrome #704
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #704 +/- ##
==========================================
+ Coverage 94.55% 94.79% +0.24%
==========================================
Files 296 298 +2
Lines 21984 22192 +208
==========================================
+ Hits 20786 21037 +251
+ Misses 1198 1155 -43 ☔ View full report in Codecov by Sentry. |
#703 It's not clear why the other tests didn't pass, I didn't change any of the other code. |
It is just some random failure - some tests in this repository are not deterministic. In general - this is not a big deal. |
I thought it was my problem..... |
In any case, the question about |
aabb theoretically outputs either aa or bb, here it only outputs bb |
I think that should be allowed. |
The code looks quite good. Please update the doc string by adding some explanation how do you find the longest palindrome, adding a sentence about non-unique solution (I guess, your function returns the one most on the right). Also, please add a test case with |
This compilation error is a machine problem? |
} | ||
let s: Vec<char> = s.chars().collect(); | ||
let n = s.len(); | ||
// dp indicates whether it is a palindrome |
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.
I have no idea what do you mean here. Maybe try to describe that dp[a][b]
is?
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.
It's a bit redundant here, dp is the string of the dynamic programming process
let n = s.len(); | ||
// dp indicates whether it is a palindrome | ||
let mut dp = vec![vec![true; n]; n]; | ||
// res record the indexes before and after the palindrome |
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.
Are you sure about the before part? I am not mistaking, the res[0]
is the index of the first character of the resulting palindrome.
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.
res[0] is the index at the beginning of the palindrome,res[1] is the index of the end of the palindrome
let mut res = (0, 0); | ||
|
||
// form filling strategy: fill in the form by length | ||
// k denotes the length of the palindrome, the further you traverse, the longer it is |
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.
Why not use use palindrome_length
instead of k
?
// strings of 2 equal characters are palindromes | ||
dp[i][i + k] = s[i] == s[i + 1]; | ||
} else { | ||
// a string that is equal on both sides and has an iambic palindrome in the middle is also an iambic palindrome |
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.
Please explain what iambic
is.
dp[i][i + k] = (s[i] == s[i + k]) && dp[i + 1][i + k - 1]; | ||
} | ||
|
||
// update palindrome length, record index |
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.
// update palindrome length, record index |
// Theoretically it is possible to return either aa or bb, | ||
// there are multiple palindromes of the same length, here only the rightmost one is returned |
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.
I am still thinking about that. In case of "aabb"
, the longest palindrome does not exist. "aa"
and "bb"
are palindromes, but they have equal length.
The longest palindrome must be unique.
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.
You have a point, I don't think it's necessary to get hung up on whether the longest is unique or not,
it's possible for a tournament to have a double winner as well
@changweidalaifu6 after rethinking the whole thing, I am not sure anymore if this contribution is suitable for this repository. The problem is very specific (it is a leetcode problem after all) and quite cumbersome to state properly (well, the function should be called something like |
It's true that it's a bit buttoned up, but it's very tight |
Updated code and added comments