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

Hash Map Lesson: Make use of terms more consistent #29008

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

softy-dev
Copy link
Contributor

@softy-dev softy-dev commented Oct 26, 2024

Because

The hash map lesson refers to some terms as being different things each time. For instance, in one line it's said that we have to "grow the buckets", when in reality what we grow is the array, or the hash table. I thought it would be welcome to correct these instances so they're consistent.

This PR

  • Modifies some paragraphs to make the terms more appropriate, so as to not confuse the students.

Issue

Closes #XXXXX

Additional Information

Discord message where a student is confused about how it's currently written.

Pull Request Requirements

  • I have thoroughly read and understand The Odin Project curriculum contributing guide
  • The title of this PR follows the location of change: brief description of change format, e.g. Intro to HTML and CSS lesson: Fix link text
  • The Because section summarizes the reason for this PR
  • The This PR section has a bullet point list describing the changes in this PR
  • If this PR addresses an open issue, it is linked in the Issue section
  • If any lesson files are included in this PR, they have been previewed with the Markdown preview tool to ensure it is formatted correctly
  • If any lesson files are included in this PR, they follow the Layout Style Guide

@github-actions github-actions bot added the Content: JavaScript Involves the JavaScript course label Oct 26, 2024
@JoshDevHub JoshDevHub self-assigned this Nov 11, 2024
@@ -98,7 +98,7 @@ This is an oversimplified explanation; we'll discuss more internal mechanics lat

Now if we wanted to get a value using a key:

1. To retrieve the value, we hash the key and calculate its bucket number.
1. To retrieve the value, we hash the key and calculate its bucket's index.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
1. To retrieve the value, we hash the key and calculate its bucket's index.
1. To retrieve the value, we hash the key and calculate the index of its bucket.

This wording is a bit more clear to me.

@@ -168,17 +168,17 @@ For example, if we are to find the bucket where the value `"Manon"` will land, t

As we continue to add nodes into our buckets, collisions get more and more likely. Eventually, however, there will be more nodes than there are buckets, which guarantees a collision (check the additional resources section for an explanation of this fact if you're curious).

Remember we don't want collisions. In a perfect world each bucket will either have 0 or 1 node only, so we grow our buckets to have more chance that our nodes will spread and not stack up in the same buckets. To grow our buckets, we create a new buckets list that is double the size of the old buckets list, then we copy all nodes over to the new buckets.
Remember we don't want collisions. In a perfect world, each bucket will either have 0 or 1 node only, so we grow our hash table to have more chance that our nodes will spread and not stack up in the same buckets. To grow our hash table, we create a new one that is double its size and then copy all existing nodes over to the buckets of this new table, hashing their keys again.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see what you're going for and why you think the current wording is a problem, but I think changing to hash table is also potentially confusing. Learners might interpret that to mean they need to create an entirely new HashMap instance and copy entries over to it somehow.

What if you called it "buckets array" in this section? This should make clear it's the containing array that needs to be changing rather than anything about individual buckets. Let me know what you think.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the feedback. I'll make the proposed changes. Sorry for the delay!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Content: JavaScript Involves the JavaScript course
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants