search

manojgop / sawtooth-poet

Apache License 2.0
1 stars 6 forks source link

Sha256 or Sha512 can be moved as utility functions #24

Open arsulegai opened 5 years ago

arsulegai commented 5 years ago

https://github.com/manojgop/sawtooth-poet/blob/aeeac4696efa1f70b104acbf82606d43a606c757/src/validator-registry-tp/src/validator_registry_tp.rs#L108

Reason is same code is written at multiple places and is potential part of code for a utility function.

askmish commented 5 years ago

Not everything should be a function.

arsulegai commented 5 years ago

Agree, but there are multiple occurrences of Sha256, Sha512 object creation, sometimes in between core logic. These objects then accept string or bytes (as per current code) to compute required hash. 3 lines are repeated at multiple places in same crate. This is same case with 'core' cargo project. That's the reason common utils was a proposal.

It would also be nice if we can think of macro here. This is for better code reusability and readability. I want to still reopen this issue as a tracker / TODO item.

askmish commented 5 years ago

Like I said earlier, not everything should be a utility function. Especially trivial things like these. I do not see any value in maintaining a separate utility function for just 2-3 lines of extra code, because of overuse of code reuse mindset. By that logic, lamdas wouldn't have existed.