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

[chore] folder/file naming conventions #457

Closed
mitchellirvin opened this issue Jul 11, 2022 · 25 comments · Fixed by #1607
Closed

[chore] folder/file naming conventions #457

mitchellirvin opened this issue Jul 11, 2022 · 25 comments · Fixed by #1607
Labels
good first issue Good for newcomers help wanted Extra attention is needed

Comments

@mitchellirvin
Copy link
Collaborator

we should pick a naming convention and update the entire repo to use that naming convention

currently for files we have a weird combination of Pascal-and-Spinal-Case

for the languages that have separated the neetcode 150 and the blind 75, we don't have a consistent convention for directory naming.

i'd propose moving to spinal-case across the board, but don't feel strongly about it. curious if anyone has strong opinions.

when we do the refactor, we should do it in chunks (maybe folder by folder) to make the PR reviews easier.

@neetcode-gh any preferences?

@mitchellirvin mitchellirvin changed the title [chore] naming conventions [chore] folder/file naming conventions Jul 11, 2022
@mitchellirvin mitchellirvin added help wanted Extra attention is needed good first issue Good for newcomers labels Jul 11, 2022
@pramitsingh0
Copy link

Hi, I would like to contribute to your work and was wondering if snake_case(words separated by underscores) will be a good fit for the issue, since you don't have a good feeling about spinal-case?

@mitchellirvin
Copy link
Collaborator Author

I'd be cool with snake case. Let's wait for @neetcode-gh to drop their two pennies before we start the refactor, though!

@pramitsingh0
Copy link

Hi. Are there any updates? Should I proceed with the work

@mitchellirvin
Copy link
Collaborator Author

Neet's probably been getting a ton of pings on this repo, it's blown up a bit since the video about the website update. It could be a while before there's a decision, I'd recommend finding other places to contribute in the meantime!

@neetcode-gh
Copy link
Owner

Thanks for looking into this! Yeah im fine with having a standard naming convention, but updating the file names will break the links on the site. The links will mostly have to be updated manually, along with the file names so I'm wondering if it's worth it. If there are any PRs that change file names, please cc me on them so I know to update the links, thanks!

@mitchellirvin
Copy link
Collaborator Author

one other thought to add to this. in some languages we're using a directory structure that maps to the categories on neetcode.io (e.g. neetcode_150/backtracking/).

we should likely decide on a single convention? In my opinion, it's easier to search w/ CMD + F if all the solutions are in one top level directory for that language (not everyone is familiar with the t shortcut on github.com). I'm not sure I see a benefit of using the nested dirs structure?

@qasim29
Copy link

qasim29 commented Jul 25, 2022

I'm not sure I see a benefit of using the nested dirs structure?

In my Opinion the nested directory structure is quite compact and easy to use it really gives a brief knowledge about the domains of DSA in a glance.

@mevimo
Copy link
Contributor

mevimo commented Jul 27, 2022

I also find it easier to CTRL/CMD + F the repo to find solutions. The website already does a good job of showcasing the problems sorted into their respective domains.

@KC10201
Copy link
Contributor

KC10201 commented Jul 27, 2022

I think a nested directory structure is more organized and less cluttered. If we need to find which directory it belongs to, we could always add a list in the Readme to indicate which category each question is under.

@pramitsingh0
Copy link

Yep! Nested Directory Structure looks well organized and much easier to navigate around

@sdkdeepa
Copy link

Hello ! Could I contribute to this?

@mitchellirvin
Copy link
Collaborator Author

mitchellirvin commented Aug 31, 2022

thanks for all of the thoughts and input, everyone!

i'm on board with snake_case and the same nested directory structure as the website (this seems like the general consensus/preference).

@Ahmad-A0 would these types of changes break the fancy new "Missing Solutions" feature in the readme (renaming files?)?

i think people are free to start work on this refactor, with these requirements:

  1. tag @neetcode-gh as the reviewer on your PR. it can't merge until he's taken the time to update the website accordingly, and for that reason he should be the only person merging these PRs
  2. also tag @Ahmad-A0 as a reviewer on the PR, so he can update the "Missing Solutions" script accordingly (this depends on whether or not these changes would actually break that feature, ahmad should be able to answer that for us)
  3. please change at most one language + problem set (blind 75 or neetcode 150) per PR. for example, you might decide to make one PR for java/blind_75 (though you're free to make smaller PRs if you'd like). if PRs get too large it's hard to ensure to quality of review can stay high. this will also help neet and ahmad's updates be reasonably scoped
  4. ensure your changes are all of the snake case form, including the appropriate subdirectory and preceding zeros, if any. e.g. java/1-Two-Sum.java would become java/neetcode_150/01_arrays_&_hashing/0001_two_sum.java

we should also update the https://github.com/neetcode-gh/leetcode/blob/main/CONTRIBUTING.md#contributing-guidelines to make sure they reflect the decided convention.

@mitchellirvin
Copy link
Collaborator Author

@Ahmad-A0 understood, thank you!

do you mind if we add preceding zeros where applicable (e.g. 0001_two_sum.java)? i think it'd be really nice to make the github file sorting add some navigability (this way problems would actually be ordered by number ascending)

@Ahmad-A0
Copy link
Collaborator

Ahmad-A0 commented Aug 31, 2022

Since the script uses the <problem-number> prefix it should be able to continue working after a quick fix to read nested directories, which I will likely push tomorrow.

However I wanted to suggest using the name leetcode assigns in URLs to avoid any confusion or discrepancies across solutions eg. 0242-valid-anagram.<extension> for https://leetcode.com/problems/valid-anagram/. This change could be incorporated with the nested directory structure, or we could maintain the flat directory structure and add an automated README component to organise the problems into problem type.

@mitchellirvin and others, thoughts?

@Ahmad-A0
Copy link
Collaborator

@Ahmad-A0 understood, thank you!

do you mind if we add preceding zeros where applicable (e.g. 0001_two_sum.java)? i think it'd be really nice to make the github file sorting add some navigability (this way problems would actually be ordered by number ascending)

Yep this is fine, preceding zeroes won't affect the script.

@mitchellirvin
Copy link
Collaborator Author

does the name in leetcode's URLs ever differ from the name in the title of the problem? I'm not sure I understand the impact of that change. what would the before/after look like?

@Ahmad-A0
Copy link
Collaborator

Ahmad-A0 commented Aug 31, 2022

The name in the URLs always matches the name of the problem, you can view the full list in the table script on the .github dir.

const URLS = { 
     'NeetCode 150'[ 
         'https://leetcode.com/problems/contains-duplicate/', 
         'https://leetcode.com/problems/valid-anagram/', 
         'https://leetcode.com/problems/two-sum/', 
         'https://leetcode.com/problems/group-anagrams/', 
         'https://leetcode.com/problems/top-k-frequent-elements/', 
         'https://leetcode.com/problems/product-of-array-except-self/', 
         'https://leetcode.com/problems/valid-sudoku/', 
         'https://leetcode.com/problems/encode-and-decode-strings/', 
         'https://leetcode.com/problems/longest-consecutive-sequence/', 
         'https://leetcode.com/problems/valid-palindrome/', 
         'https://leetcode.com/problems/two-sum-ii-input-array-is-sorted/', 
         'https://leetcode.com/problems/3sum/', 
         'https://leetcode.com/problems/container-with-most-water/', 
         'https://leetcode.com/problems/trapping-rain-water/', 
         'https://leetcode.com/problems/best-time-to-buy-and-sell-stock/', 
         'https://leetcode.com/problems/longest-substring-without-repeating-characters/', 
         'https://leetcode.com/problems/longest-repeating-character-replacement/', 
         'https://leetcode.com/problems/permutation-in-string/', 
         'https://leetcode.com/problems/minimum-window-substring/', 
         'https://leetcode.com/problems/sliding-window-maximum/', 
         'https://leetcode.com/problems/valid-parentheses/', 
         'https://leetcode.com/problems/min-stack/', 
         'https://leetcode.com/problems/evaluate-reverse-polish-notation/', 
         'https://leetcode.com/problems/generate-parentheses/', 
         'https://leetcode.com/problems/daily-temperatures/', 
         'https://leetcode.com/problems/car-fleet/', 
         'https://leetcode.com/problems/largest-rectangle-in-histogram/', 
         'https://leetcode.com/problems/binary-search/',

Visually the layout would look pretty similar to what we have now without the casing issues and occasional symbol usage in problem names. Something like this could also be automated since I already have the problem numbers linked to the URLs.

@mitchellirvin
Copy link
Collaborator Author

that's a good idea.

i'd prefer to keep snake case for consistency (as opposed to spinal case), though. e.g. 0242_valid_anagram.<extension> just so we're not mixing and matching naming conventions

@Ahmad-A0
Copy link
Collaborator

Yep, that sounds great. I can write a script to rename the files although I don't know how practical that is until something like #763 gets done.

@Ahmad-A0
Copy link
Collaborator

I'm wondering if the nested directory structure is still necessary with the changes made to the README to reflect problem category, having a different folder for the <problem-set> could especially be problematic as some problems would now appear three times in the NeetCode 150, Blind 75 and NeetCode All lists.

@neetcode-gh
Copy link
Owner

neetcode-gh commented Dec 22, 2022

This is long overdue, but I'll have some time over the next week to implement dynamic URLs from #763, so I'm happy to review and merge any PRs related to renaming/organizing. I'll time it to merge the PRs the same time I push my changes.

The main thing I would need is to be able to know if a solution exists for a given language (e.g. LC 1 has a solution in Java). But the readme table that @Ahmad-A0 implemented pretty much solves that, but I would just need to convert it to JSON. Btw is there a script you're using for that? At first glance i couldnt find it.

This will also result in adding many more solutions & lanugages to the site!

@Ahmad-A0
Copy link
Collaborator

@neetcode-gh

I'm currently using the updateCompletionTable.js script hidden away in ./.github/workflows, it just grabs the problem number for each leetcode question, which I have stored in this JSON file, and sees if the language folder e.g c, cpp, python, etc. has any solutions that start with that number.

Depending on how you want to implement this on your side it might be helpful for me to add another JSON file to this repo that keeps track of which problems are completed alongside the table.

@neetcode-gh
Copy link
Owner

Thanks, I'm almost done with the site changes required. Do you know if there's already some open PRs for doing the renaming? If not, i can do them.

@Ahmad-A0
Copy link
Collaborator

There are no open PRs! Unrelated but, can I also ask how files are found on the site's end so I don't break things in the future. @neetcode-gh

@neetcode-gh
Copy link
Owner

@Ahmad-A0 Just opened #1607 - the PR has a file (.problemSiteData.json) which is the file that contains code links for the site. Lmk if you have any questions, and thank you!

@neetcode-gh neetcode-gh linked a pull request Dec 27, 2022 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants