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: 238-Product-Of-Array-Except-Self.js #1579

Merged
merged 1 commit into from
Dec 26, 2022

Conversation

benmarg
Copy link
Collaborator

@benmarg benmarg commented Dec 23, 2022

@benmarg
Copy link
Collaborator Author

benmarg commented Dec 23, 2022

Refactor to be more similar to the Python solution, eliminate clutter from the other overlapping solutions.

@aakhtar3
Copy link
Collaborator

This is a duplicate solution while losing all the content to help people learn multiple complexity solutions.

@benmarg
Copy link
Collaborator Author

benmarg commented Dec 23, 2022

The idea was to simplify the solution and get it more in line with the Python solution. IMO a current problem with the Javascript solutions in general is that they are way too verbose. And I don't necessarily think providing more solutions is always better. I think their's a lot of value in being able to click the JS button and see one clear and concise solution. It makes it way easier to focus and process what's going on in the given solution.

Sometimes maybe their's a really good reason to include multiple solutions(for example showing an iterative vs recursive approach is great IMO). Or if their is a tradeoff between Time vs Space, showing both versions in that case also makes a lot of sense.

Expanding on the "verbose" point, I'm not sure why a lot of these JS solutions end up using like 4 helper functions and 40 LOC for something in the Python version that's just one ten-line function.

That's basically what I was trying to highlight in this PR. If this is something that the maintainers have considered and decided that they prefer it this way, then I totally understand leaving things as they are. Or if maybe my approach would work with some tweaking? Thoughts?

@aakhtar3
Copy link
Collaborator

aakhtar3 commented Dec 23, 2022

We can add this simple solution to this file at the top or bottom of file, but it is essential the same as the Space O(1) solution.

Neetcode is a resource to help developers learn algorithms and data structures. Solutions that are too concise or with non descriptive variables/helper functions are harder to retain common algorithm patterns. Modular code is also seen as a bonus when doing real interviews, where as a giant function tend to get messy.

A few other collaborators appreciate this approach by being verbose, providing descriptive functions (some helper functions are repeated across a many solutions i.e. carry forward/backward, get neighbors, etc), and complexities (#213).

@benmarg
Copy link
Collaborator Author

benmarg commented Dec 24, 2022

Hmm so while I'm totally on board with the fundamental principle you mentioned, helping developers learn DSA, I'm not sure that the "verbose and diverse" approach is really the best way to achieve that.

I think that it's important to to understand the unique thing that makes Neetcode.io as amazing as it is: the video explanations by NC. If someone wants to see a solution for a given LC problem, they can just go to the Discussion/Solution tab on LC.com and sort by my post popular, and find a solution that they can use to understand the problem. Essentially duplicating that functionality isn't really playing to the strengths of NC.

In order to really make the JS/C++/Java solutions a powerful tool that is tailored to the NC platform, the solutions should be tailored to following use case IMO: a dev who's primary language isn't Python, who watched the NC video and is still trying to wrap their head around the problem, or just wants to see the solution in their primary lanugage.

The best way to do that is by making those other solutions as similar to the Python solution as possible, so the person learning has a much easier context shift from trying to understand the solution as presented by NC, to trying to understand the solution in their preferred language. If the more verbose/full of helper functions solution is really that much better, why isn't the Python solution implemented like that? Don't get me wrong, helper functions have a place and time, NC himself pulls them out when appropriate. But adding them for "clarity's sake" I think actually does the opposite of making the pattern easier to retain. You're turning what could be an easy to look at 10 line solution, into a 40 line monstrosity with multiple interwoven parts, where understanding how they all fit together becomes needlessly difficult.

So like for the o(n) space complexity solution for this problem, what is the idea behind keeping that for example? Not only is it's worse space/time then the 2nd solution, it's also way more lines of code. How does including that help someone learn the problem?

@aakhtar3
Copy link
Collaborator

aakhtar3 commented Dec 24, 2022

Yeah that's a good idea to have a python parity solution. It can be put at the top so that the other solutions don't get lost.

Searching for JS solutions on leetcode isn't that great. Having multiple complexities solutions is helpful to see the progression of improving complexities and the identifying the trick or better DS.

@aadil42
Copy link
Contributor

aadil42 commented Dec 24, 2022

@aakhtar3, @benmarg Yeah, We should have the python solution implemented in JS. Also, in the video do-while loop is rarely used. Can we use a do-while loop in our solution, or is it a bad practice?

@benmarg
Copy link
Collaborator Author

benmarg commented Dec 25, 2022

Could you clarify what you mean @aadil42? If I understand what you're saying, you're just asking to use more while loops in the solutions? I mean that's kind of something to take up with Neetcode, it's not bad practice, they just have their time and place and when needed he does implement them, for example: Maximum Depth of Binary Tree.

That said it's kinda of the opposite of the idea that I'm pushing, which is that the JS solution should be as similar as possible to the Python solution. Even if a while loop could be cleaner, the solution should still use a for loop if that's what NC used in his solution, for all the reasons provided earlier in the thread.

@aadil42
Copy link
Contributor

aadil42 commented Dec 25, 2022

No, I meant the do-while loop. I know he uses while loops.

@benmarg
Copy link
Collaborator Author

benmarg commented Dec 26, 2022

I talked to Neetcode @aakhtar3. He agrees that it's fine to just have one clean Python-esq solution, keeping the other solutions ends up being a bit redundant. He also added me as a collaborator. I'm going to merge this solution start going through other JS problems to clean them up. Thanks for all the input!

@benmarg benmarg merged commit 97219d8 into neetcode-gh:main Dec 26, 2022
@benmarg benmarg deleted the productExceptSelf branch December 26, 2022 00:32
@aadil42
Copy link
Contributor

aadil42 commented Dec 26, 2022

That's great!

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.

3 participants