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

Added hash map tests for Ruby #28263

Merged

Conversation

elishamutang
Copy link
Contributor

Because

Allows users to populate their hash maps where it will exceed their load factor and ensuring that their hash map methods will still work as expected after growing their buckets (similar to this PR for JavaScript course).

This PR

Added a new section to test out user hash map implementation after Assignment section.
Added dummy nodes to populate user hash map in the new section.

Issue

Closes #28257

Additional Information

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: Ruby Involves the Ruby course label Jun 22, 2024
@SumonGFC
Copy link
Contributor

Hello :p Small gripe: In ruby style conventions, the names of "predicate methods" (methods that return a boolean) should end with a question mark to indicate that they return booleans. So on lines 52 and 109 in your proposed changes, #has(key) should be named #has?(key). just wanted to let you know.

@elishamutang
Copy link
Contributor Author

@SumonGFC Ah i see, didn't know that. Thanks for letting me know, I'll undo that change! What about the the set method in lines 44 & 48?

@SumonGFC
Copy link
Contributor

SumonGFC commented Jun 23, 2024

@elishamutang those lines are perfect. This set method is not supposed to return a boolean in the interface described by this project (think it is supposed to return the value that is associated with the key)

So #has?(key) is the only method that should have the question mark on the end

Copy link
Contributor

@JoshDevHub JoshDevHub left a comment

Choose a reason for hiding this comment

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

Awesome work 🙌

Like @SumonGFC said, the only suggestion I have is to append a ? to the #has method name (I forgot to mention this as something to keep in mind).

After that's fixed, we should be good to merge.

ruby/computer_science/project_hash_map.md Outdated Show resolved Hide resolved
@JoshDevHub JoshDevHub self-assigned this Jun 24, 2024
Co-authored-by: Josh Smith <[email protected]>
@elishamutang
Copy link
Contributor Author

@JoshDevHub totally missed that last #has method haha. No worries and thanks for the help! You too @SumonGFC :)

Copy link
Contributor

@JoshDevHub JoshDevHub left a comment

Choose a reason for hiding this comment

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

Thanks for doing this!

@JoshDevHub JoshDevHub merged commit 762c2cb into TheOdinProject:main Jun 25, 2024
3 checks passed
@elishamutang elishamutang deleted the hashmap_implementation_examples branch June 25, 2024 16:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Content: Ruby Involves the Ruby course
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ruby: HashMap Project: Add new section to test user hash map
3 participants