2
\$\begingroup\$

Is there a neater way of handling the option response from req.cookie() in this code block (line 3 onwards)? Id like to avoid the nested if statements, and multiple returns if at all possible. req.cookie() returns an Option<Cookie> and the Cookie struct has a value function. If the cookie is present but has the wrong value, i want to return a bad request, however i also want to return a bad request if the cookie is not present at all. If the cookie value is OK, i want to continue and do further work:

#[get("/callback")]
async fn callback(req: HttpRequest, client: web::Data<BasicClient>, web::Query(cb): web::Query<Callback>) -> Result<HttpResponse> {
    if let Some(state_cookie) = req.cookie(STATE_COOKIE_NAME) {
        if state_cookie.value() != cb.state {
            return Ok(HttpResponse::BadRequest().finish())
        }
    } else {
        return Ok(HttpResponse::BadRequest().finish())
    }

    let token_result = client.exchange_code(AuthorizationCode::new(cb.code)).request(http_client);

    return match token_result {
        Ok(token_details) => Ok(handle_success(token_details)),
        Err(token_error) => Ok(handle_error(token_error))
    }
}
\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

There is a way without multiple returns and nested if statements.

You have three control flow pathways in your program, related to three states of state_cookie respectively:

  • cookie is Some and state_cookie.value() == cb.state
  • cookie is Some and state_cookie.value() != cb.state
  • cookie is None

For the first, we proceed, whereas for second and third we branch to do the same return. Do the 2. and 3. have anything in common? They cover all except one value.

You may be coming from a language where data types lack expressiveness. Many languages don't have Option at all. In Rust, we have deep operations on Algebraic Data Types like Option.

To postpone comparison and branching until we have all information, we do a map on the Option: req.cookie(STATE_COOKIE_NAME).map(|sc| sc.value()). Then, a simple comparison divides our function into two control flow paths. As an extra, for self-documenting code, we make a short-lived binding for state_cookie_value:

#[get("/callback")]
async fn callback(req: HttpRequest, client: web::Data<BasicClient>, web::Query(cb): web::Query<Callback>) -> Result<HttpResponse> {
    let state_cookie_value = req.cookie(STATE_COOKIE_NAME).map(|sc| sc.value());
    if state_cookie_value != Some(cb.state) {
        return Ok(HttpResponse::BadRequest().finish());
    }

    let token_result = client.exchange_code(AuthorizationCode::new(cb.code)).request(http_client);

    match token_result {
        Ok(token_details) => Ok(handle_success(token_details)),
        Err(token_error) => Ok(handle_error(token_error))
    }
}

This is basically all in this code.

I believe this may obscure the code's meaning for the reader, but you can as well write the match statement as:

Ok(token_result.map_or_else(handle_error, handle_success))

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.