One-shot APIs -- static functions vs methods
Maintainers usually reply within 2 days
Nobody has claimed this yet.
Assessment
- Difficulty
- 5/5
- Estimated time
- Over a week
- Newbie friendliness
- 45/100
Research direction
Start by inspecting the Hash trait and HashFactory, then search the repository for other traits using the ::new().hash(..) pattern. Determine whether the factory can support static ::hash(..) calls while remaining dyn-compatible; if not, apply the proposed dual-method approach. Done means similar patterns are cleaned up and crate documentation uses the static form.
Written by the indexing model from the issue text.
Description
Consider:
let output: Vec<u8> = sha3::SHA3_256::new().hash(data);
The pattern of ::new().hash(..), where .hash(..) is a method, is a code smell since should really be a static function ::hash(..). The reason why it's like that is because the HashFactory requires the Hash trait to be dyn-compatible, and I was not able to get the factory to work with a static ::hash(..).
The task for this ticket is to play some more with whether it's possible to get the HashFactory to chain properly to a ::hash(..). If not, @npajkovsky suggested that we could cheat and simply have the Hash trait have both versions:
trait Hash {
fn hash(data: &[u8]) -> Vec<u8> {
Self::new().hash_method(data)
};
fn hash_method(self, data: &[u8]) -> Vec<u8>;
}
and then we continue to use the .hash_method(data) version within the HashFactory, but we can simplify the sample code in the crate docs to use the cleaner SHA256::hash(data) version.
Note: I'm using Hash as an example, but this smelly pattern exists across other traits as well. This ticket should clean up all similar patterns.
- Dominant language
- Rust
- Stars
- 25
- Forks
- 18
- Avg merge
- 14d 10h
- Merged PRs (30d)
- 5
Getting set up
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
More from bcgit/bc-rust
-
documentation good first issue help wanted
Difficulty 2/5 1-3 hours Newbie friendliness 82/100
bcgit/bc-rust#161 · 1 reaction ·
Maintainers usually reply within 2 days
-
good first issue refactor
Difficulty 2/5 1-3 hours Newbie friendliness 72/100
Maintainers usually reply within 2 days
-
Difficulty 5/5 Over a week Newbie friendliness 25/100
bcgit/bc-rust#163 · 2 comments ·
Maintainers usually reply within 2 days
-
Difficulty 4/5 3-5 days Newbie friendliness 35/100
Maintainers usually reply within 2 days
-
documentation good first issue refactor
Difficulty 3/5 1-2 days Newbie friendliness 55/100
Maintainers usually reply within 2 days
Similar issues
-
area:release bug
Difficulty 2/5 1-3 hours Newbie friendliness 86/100
registrystack/registry-stack#1874 ·
Maintainers usually reply within 1 day
-
component:midnight-toolkit status:untriaged
Difficulty 2/5 1-3 hours Newbie friendliness 72/100
midnightntwrk/midnight-node#2237 ·
Maintainers usually reply within 1 day
-
Difficulty 2/5 1-3 hours Newbie friendliness 88/100
Maintainers usually reply within 1 day
-
Difficulty 2/5 1-3 hours Newbie friendliness 78/100
Maintainers usually reply within 1 day
-
Difficulty 1/5 Under an hour Newbie friendliness 78/100
Maintainers usually reply within 1 day