-
Notifications
You must be signed in to change notification settings - Fork 5k
3sum 4sum #497
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
3sum 4sum #497
Conversation
|
@remlostime I initially just wanted to leave a few comments, but I ended up doing a full blown edit :] I wanted to give you some feedback based on what I saw:
I personally avoid asking the reader questions in my writing, but it's fine once in a while. 4 times in a short paragraph is a bit too much, and berates readers who weren't too savvy in connecting the dots, like myself :]
I'm a little confused by your explanation in avoiding duplicates. You wrote:
Correct me if I'm wrong, but it implied to me that I should avoid using Since the -1 values are together, Anyhow, thanks for the PR. So far, I've done editing for the 3Sum via aeffa0a. Once you've addressed my concern on the "avoiding duplicates" part, could revisit your 4Sum writeup and rewrite it based on the changes I've made to 3Sum? Thanks! |
|
@remlostime There's a cool editor called the Hemmingway editor that really helped my writing in the past. Consider checking it out when you do article writing: http://hemingwayapp.com |
|
@kelvinlauKL Thanks a lot for the awesome editing and writing app! We have three pointers |
|
@remlostime I tried to simplify the writing a bit. I'm still a little confused. Here's an example:
Would we have multiple |
66a27d8 to
6ddd0cd
Compare
|
@kelvinlauKL In that cases, we will have multiple For example, The answer should be two |
|
Ok, thanks for the clarification. I'll continue with this review (could you also squash those merge conflicts please). |
51d4a64 to
39644a8
Compare
|
Update the PR and fix the conflict. @kelvinlauKL |
|
@remlostime I think I found a bug: While you're at it, could you rename the variables of your code to |
Follow up 2Sum problem. How to solve 3Sum and 4Sum problem? What’s their relationships? Could we get some generic idea to solve this kind of problems? I give the explanation here.
Follow up 2Sum problem. How to solve 3Sum and 4Sum problem? What’s their relationships? Could we get some generic idea to solve this kind of problems? I give the explanation here.
Work in progress, not done yet!
|
@kelvinlauKL I looked in to code again. And find this if l != 0 && a[l] == a[l-1] {
l += 1
flag = true
}
if (r != a.count - 1) && a[r] == a[r+1] {
r -= 1
flag = true
}So, I think that's why you see the unique result. Since this PR is pretty long time ago, I just recall that should make sense to just keep unique answers. Sorry for the confusing. PR has been updated. |
|
@remlostime Works for me! |
Easier to understand which parts of the code is borrowed using TwoSum.
Seems like a good use case for `defer`... giving the index traversal more visibility by putting it next to the `while` loop. Reads a bit more like for loops now, where exit condition and traversal procedure can be understood in 1 line.
|
@remlostime Merging this in :) Thanks! |
|
Thanks for the awesome edit!!! @kelvinlauKL |
Checklist
Description
Follow up 2Sum problem. How to solve 3Sum and 4Sum problem? What’s their relationships? Could we get some generic idea to solve this kind of problems? I give the explanation here.