Avoid unwrap()
#45
Avoid unwrap()
#45
Conversation
| match users.filter(name.eq(target_name)).load(connection) { | ||
| Ok(results) => results.collect(), | ||
| Err(_) => vec![] | ||
| } |
There was a problem hiding this comment.
Let's just change the return type of the function to QueryResult<Vec<(...)>>. I think this match distracts from the example too much.
There was a problem hiding this comment.
QueryResult<Vec<(...)>> is awkward here because load returns QueryResult<Cursor<_,_>>.
Would the following be okay instead?
fn users_with_name(connection: &Connection, target_name: &str)
-> Option<Vec<(i32, String, Option<String>)>>
{
use self::users::dsl::*;
users.filter(name.eq(target_name)).load(connection)
.ok()
.map(|x| x.collect())
}There was a problem hiding this comment.
Why not this?
fn users_with_name(connection: &Connection, target_name: &str)
-> QueryResult<Vec<(i32, String, Option<String>)>>
{
use self::users::dsl::*;
users.filter(name.eq(target_name)).load(connection)
.map(|x| x.collect())
}
|
In theory we shouldn't be ignoring the result of delete statements. It's technically fine, since we're asserting it was successful one line down, but I don't know if encouraging not checking results is better than encouraging unwrapping. At least this results in a compiler warning. |
|
I wrapped the delete statements in |
| @@ -82,12 +88,18 @@ pub fn update<T: UpdateTarget>(source: T) -> IncompleteUpdateStatement<T> { | |||
| /// # } | |||
| /// # | |||
| /// # fn main() { | |||
| /// # delete(); | |||
| /// # () | |||
There was a problem hiding this comment.
Of course, silly me.
|
Looks good other than that comment. Can you squash the commits? |
|
All squashed, thanks for your feedback! |
This addresses #34 partially.
There are a few cases that I wasn't sure how to handle:
It seems to me that in those cases one could return an empty iterator.
I was also unsure whether I could really ignore the result of delete statement executions. That's one reason I put those changes in a separate commit.
If nothing else, this pull request locates occurrences of
unwrap()in the current documentation.