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.

Sign up for GitHub

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

Jump to bottom

std::hash docs may lead to confusion #40498

Closed
projektir opened this issue Mar 14, 2017 · 3 comments
Closed

std::hash docs may lead to confusion #40498

projektir opened this issue Mar 14, 2017 · 3 comments
Assignees
@frewsxcv
Labels
A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools

Comments

@projektir
Copy link
Contributor

https://doc.rust-lang.org/std/hash/

The example given tries to demonstrate the use of a SipHasher as well as its hash() function. But in doing so, it also creates a new function to display the resulting hash, and this new function is also called hash(). The new function returns a value, while the library hash() function does not.

Since many people expect a hash function to return a value, some confusion may occur due to the example function being named exactly the same as the built-in function while being used to do something many may expect (returning a value). This actually came up on IRC.

I suggest the function in the example should be called something other than hash(), maybe get_hash().

[alternatively, should the built-in function be called hash, since it's just collecting values and the hashing is done upon finish()?]

Another thing: SipHasher appears to be deprecated for direct use. Perhaps this example should use DefaultHasher instead? May be related to #29357.
@frewsxcv frewsxcv self-assigned this Mar 14, 2017
frewsxcv added a commit to frewsxcv/rust that referenced this issue Mar 14, 2017
@frewsxcv
A few improvements to the core::hash top-level docs.
5cc056a 
Primarily opened to address the concerns brought up in
rust-lang#40498.

* run rustfmt on code blocks
* use `DefaultHasher` instead of deprecated `SipHasher`
* rename `hash` to `calculate_hash` to prevent confusion with the `hash`
  method
@frewsxcv frewsxcv mentioned this issue Mar 14, 2017
Merged
@frewsxcv
Copy link
Member

Thanks for the suggestions! Opened a pull request: #40505

frewsxcv added a commit to frewsxcv/rust that referenced this issue Mar 15, 2017
@frewsxcv
Rollup merge of rust-lang#40505 - frewsxcv:hash-docs, r=alexcrichton
cc1e513 
A few improvements to the `core::hash` top-level docs.

Primarily opened to address the concerns brought up in
rust-lang#40498.

* run rustfmt on code blocks
* use `DefaultHasher` instead of deprecated `SipHasher`
* rename `hash` to `calculate_hash` to prevent confusion with the `hash`
  method
frewsxcv added a commit to frewsxcv/rust that referenced this issue Mar 17, 2017
@frewsxcv
Rollup merge of rust-lang#40505 - frewsxcv:hash-docs, r=alexcrichton
5c3e066 
A few improvements to the `core::hash` top-level docs.

Primarily opened to address the concerns brought up in
rust-lang#40498.

* run rustfmt on code blocks
* use `DefaultHasher` instead of deprecated `SipHasher`
* rename `hash` to `calculate_hash` to prevent confusion with the `hash`
  method
frewsxcv added a commit to frewsxcv/rust that referenced this issue Mar 17, 2017
@frewsxcv
Rollup merge of rust-lang#40505 - frewsxcv:hash-docs, r=alexcrichton
7459aa8 
A few improvements to the `core::hash` top-level docs.

Primarily opened to address the concerns brought up in
rust-lang#40498.

* run rustfmt on code blocks
* use `DefaultHasher` instead of deprecated `SipHasher`
* rename `hash` to `calculate_hash` to prevent confusion with the `hash`
  method
frewsxcv added a commit to frewsxcv/rust that referenced this issue Mar 17, 2017
@frewsxcv
Rollup merge of rust-lang#40505 - frewsxcv:hash-docs, r=alexcrichton
a463ba8 
A few improvements to the `core::hash` top-level docs.

Primarily opened to address the concerns brought up in
rust-lang#40498.

* run rustfmt on code blocks
* use `DefaultHasher` instead of deprecated `SipHasher`
* rename `hash` to `calculate_hash` to prevent confusion with the `hash`
  method
frewsxcv added a commit to frewsxcv/rust that referenced this issue Mar 17, 2017
@frewsxcv
Rollup merge of rust-lang#40505 - frewsxcv:hash-docs, r=alexcrichton
7a71d64 
A few improvements to the `core::hash` top-level docs.

Primarily opened to address the concerns brought up in
rust-lang#40498.

* run rustfmt on code blocks
* use `DefaultHasher` instead of deprecated `SipHasher`
* rename `hash` to `calculate_hash` to prevent confusion with the `hash`
  method
frewsxcv added a commit to frewsxcv/rust that referenced this issue Mar 17, 2017
@frewsxcv
Rollup merge of rust-lang#40505 - frewsxcv:hash-docs, r=alexcrichton
f47a377 
A few improvements to the `core::hash` top-level docs.

Primarily opened to address the concerns brought up in
rust-lang#40498.

* run rustfmt on code blocks
* use `DefaultHasher` instead of deprecated `SipHasher`
* rename `hash` to `calculate_hash` to prevent confusion with the `hash`
  method
@steveklabnik steveklabnik added the A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools label Mar 23, 2017
@steveklabnik
Copy link
Member

#40505 was merged so I'm closing this, please let me know if that's wrong @projektir

@projektir
Copy link
Contributor Author

Nah, looks great. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@frewsxcv frewsxcv

Labels
A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@steveklabnik @frewsxcv @projektir