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

Player Odds Endpoint #9

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

brh28
Copy link

@brh28 brh28 commented Apr 20, 2023

Includes an endpoint that returns the probability of each guess being selected from a uniformly distributed nonce.

Copy link
Owner

@notmandatory notmandatory left a comment

Choose a reason for hiding this comment

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

Overall looks good and I used curl to test the API and returned values are as expected:

 curl http://localhost:8081/api/guesses/odds
[["alice",0.6687994],["steve",0.011756565],["bob",0.31944403]]

After the suggested fixes you should also cargo fmt and squash the commits into one commit.

@@ -4,6 +4,10 @@ pub use serde;
pub use serde_json;
pub use serde_with;

const NONCE_MIN: u32 = 0;
Copy link
Owner

@notmandatory notmandatory May 3, 2023

Choose a reason for hiding this comment

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

I don't think we need constants for these, can create a type alias for u32:

Suggested change
const NONCE_MIN: u32 = 0;
type Nonce = u32;

And replace: NONCE_MIN with Nonce::MIN, NONCE_MAX with Nonce::MAX, and NONCE_SET_SPACE is always going to be Nonce::MAX.

@@ -212,6 +214,13 @@ async fn get_target_nonce(Extension(state): Extension<Arc<State>>) -> Result<Str
Ok(String::default())
}

async fn get_guess_probabalities(Extension(state): Extension<Arc<State>>) -> Result<Json<Vec<(String, f32)>>, Error> {
Copy link
Owner

Choose a reason for hiding this comment

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

little spelling typo:

Suggested change
async fn get_guess_probabalities(Extension(state): Extension<Arc<State>>) -> Result<Json<Vec<(String, f32)>>, Error> {
async fn get_guess_probabilities(Extension(state): Extension<Arc<State>>) -> Result<Json<Vec<(String, f32)>>, Error> {

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants