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

Proposal: BodyParser Should Change Contract #73

Closed
craftytrickster opened this issue Sep 9, 2016 · 0 comments
Closed

Proposal: BodyParser Should Change Contract #73

craftytrickster opened this issue Sep 9, 2016 · 0 comments

Comments

@craftytrickster
Copy link

Currently body parser works by returning a Result<Option>:

    let struct_body = req.get::<bodyparser::Struct<MyStructure>>();
    match struct_body {
        Ok(Some(struct_body)) => println!("Parsed body:\n{:?}", struct_body),
        Ok(None) => println!("No body"),
        Err(err) => println!("Error: {:?}", err)
    }

I believe that adding an Option to the return type forces the consumer to have an extra level of matching for no gained benefit (in my personal opinion).

If I am trying to parse a body and there is no body present, I would argue that it should be an Err, instead of being Ok(None), after all, I was expected a value to be there that is missing. You can add MissingBody as a potential error code.

I believe this would simplify the API and make it easier to consume.

If the maintainers of this repo agree with my proposal, I can go ahead and submit a PR.

Thanks.

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

No branches or pull requests

1 participant