2
\$\begingroup\$

I've created a simple algorithm, implemented it in Rust, and published it as a crate on crates.io.
https://crates.io/crates/randevu

My code seems pretty clean and idiomatic to me, but I'd like to know if you have suggestions to improve it further, including tests and documentation.
I want to allow users to pass a chrono date/time to the function instead of the function only taking the date as a string.
I'm also considering making a version that accepts blake3 hashes of object and date (so they can be calculated only once for instances where the function is called many times for a specific object or date).

For those interested in the function and idea behind the algorithm itself, I've tried to explain it the best I can in the README.md (though it might not be relevant for code review).

//! The official Rust implementation of the [RANDEVU](https://github.com/TypicalHog/randevu) algorithm //! //! # Example //! ```rust //! use randevu::{rdv, utc_date_with_offset}; //! //! fn main() { //! let object = "THE_SIMPSONS"; //! let date = utc_date_with_offset(0); //! let rdv = rdv(object, &date); //! //! println!("Object {} has RDV{} today", object, rdv); //! } //! ``` use blake3; use chrono::{TimeDelta, Utc}; /// Returns current UTC DATE `String` in ISO 8601 format (YYYY-MM-DD), with an OFFSET `i64` in days pub fn utc_date_with_offset(offset: i64) -> String { (Utc::now() + TimeDelta::days(offset)) .format("%Y-%m-%d") .to_string() } /// Returns RDV level `u32` for an OBJECT `&str` on a specific DATE `&str` /// /// **RDV = number of leading zero bits in blake3(blake3(OBJECT) || blake3(DATE))** pub fn rdv(object: &str, date: &str) -> u32 { let mut hasher = blake3::Hasher::new(); hasher.update(blake3::hash(object.as_bytes()).as_bytes()); hasher.update(blake3::hash(date.as_bytes()).as_bytes()); let final_hash = hasher.finalize(); let mut rdv = 0; for &byte in final_hash.as_bytes() { rdv += byte.leading_zeros(); if byte != 0 { break; } } rdv } #[cfg(test)] mod tests { use super::*; #[test] fn test_rdv_1() { assert_eq!(rdv("NO_BOILERPLATE", "2024-01-26"), 11); } #[test] fn test_rdv_2() { assert_eq!(rdv("SHREK_2001", "2024-01-26"), 8); } #[test] fn test_rdv_3() { assert_eq!(rdv("RANDEVU", "2024-01-26"), 1); } #[test] fn test_rdv_4() { assert_eq!(rdv("RUST", "2024-01-26"), 0); } }
\$\endgroup\$

1 Answer 1

Reset to default
4
\$\begingroup\$

Naming

The function rdv has a name, that I'd never remember. It's described as number of leading zero bits, but it is never explained where the presumable abbreviation originates from.

offset: i64 is passed to TimeDelta::days. So you should either explain that those are days in the docstring or rename the variable to days, so that the user knows what it does.

Aside from object being a non-descriptive name in general, object: &str is not generic over some type T. It always will be a &str. Since something called an RDV level is being calculated from it, maybe call it target or so. I really don't know since I cannot make sense of rdv() at all.

Get your types early

rdv() should receive date as an appropriate object, not as a string slice. The serialization should be done internally. With the current implementation one can pass in an arbitrary string, that may not meet the function's expectations of a valid date.

E.g. what would you expect x to be in assert_eq!(rdv("NO_BOILERPLATE", "Hello world"), x);?

If you want to make rdv() really generic, as claimed in the README, you could rewrite it to rdv<T: ToString>(target: T, ...)....

Documentation

The contents of the project's README file are misleading.

  1. The algorithms provided are not generic, as noted above.
  2. Nothing is being done on a daily basis - there is no timed automation whatsoever in the code. It just exhibits some date serialization and a spurious hash function.
\$\endgroup\$
8
  • \$\begingroup\$ Thank you for your time and answer! The function is named rdv, which is short for RANDEVU (name of the algorithm). You can think of it the same way as ETA stands for "estimated time of arrival" or BCE which stands for "before common era". I guess I should explain that better in the README.md file. The values generated by the algorithm are commonly prefixed with RDV when one is referring to them and using them for the intended usecases. If an object has a value 5 on a given day, the convention is to refer to it as RDV5. Example usage: "Hey Mike, XONOTIC has RDV5 today, wanna play?" \$\endgroup\$ Commented Mar 5, 2024 at 19:13
  • \$\begingroup\$ I do explain the offset is in days in the docstring: /// Returns current UTC DATE String in ISO 8601 format (YYYY-MM-DD), with an OFFSET i64 in days I see how my use of the word generic can be confused with the type being generic. The object is ALWAYS a string, but that string can represent ANY object (such as a movie, game, person, video, book, place, post, task etc.) So I call it generic since the algorithm can be applied to ANYTHING that can be represented with a string as opposed to just tasks or meetings etc. I do need to improve the README and explain stuff better. \$\endgroup\$ Commented Mar 5, 2024 at 19:16
  • \$\begingroup\$ The rdv function accepts a string and not a date because it is assumed the date will be stored in a file or somewhere else as a string, so it would have to be converted back to date and then back to string again before it is hashed. I guess I should add a check which would reject invalid dates and maybe allow users to either pass the date as a string or a date object itself to the function. \$\endgroup\$ Commented Mar 5, 2024 at 19:17
  • 1
    \$\begingroup\$ "so it would have to be converted back to date and then back to string again before it is hashed" <- which is perfectly fine, because rdv will be cleaner and easier to maintain (and use) as a result. If you change the type of rdv's date parameter to be a date object, you won't need to write validation logic in the body of rdv, and the return type can remain as it is, u32. If you keep date as a string and add validation logic, you'll need to change the return type to a Result or allow rdv to panic, both which are not as clean alternatives compared to just using a date object. \$\endgroup\$ Commented Mar 7, 2024 at 2:12
  • 1
    \$\begingroup\$ @Setris I've made another post related to this (just in case your are interested, I don't wish to leech your time): codereview.stackexchange.com/questions/291897/… \$\endgroup\$ Commented May 3, 2024 at 20:47

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.