How to break the function map()?

[mhanusek]

Hello!

I have this function:

fn map_to_enums(strings: &Vec<&str>) -> Result<Vec<MyEnum>, String>
{
    let enums_vec : Vec<_> = strings.iter().map(|s| { 
            let e = MyEnum::try_from(&s.to_string()).unwrap();
            //TODO: if Error => return Error
            e
    }).collect();
    
    return Ok(enums_vec);
}

How to break the function map if the function try_from returns an error?
Maybe I should use a different function instead of the map()?

My example:
Playground

[alice]

You can pass on the Result as the value in the iterator. The collect function then supports collecting in to a Result<Vec<T>, E> instead of collecting to an Vec<T> directly.

let enums_vec = strings.iter().map(|s| { 
    MyEnum::try_from(&s.to_string())
}).collect::<Result<Vec<_>, _>>()?;

[H2CO3]

Three more comments:

• It looks like you are trying to parse a string for creating an enum. Are you sure you should take an allocating String by-value? Shouldn't you parse the already-available &str for parsing? Also, if this is indeed what you are trying to do, consider implementing FromStr alongside or instead of TryFrom. That trait is the idiomatic trait for parsing.
• Don't use explicit returns at the end of the function. Writing Ok(enums_vec) is enough.
• Don't take &Vec<_> as a parameter. It is useless: anything you can with an immutable reference to a Vec you can do with an immutable reference to a slice, &[T]. Accepting &Vec<T> is actually worse: if you have something that coerces to a slice, it isn't generally convertible to a &Vec<T>, only to a slice. So you will force the consumer of the code to perform unnecessary immediate allocations in order to convert to a temporary Vec.

[steffahn]

@H2CO3

Three more comments

In other words: Use clippy

[Clippy output (click to expand)]

    Checking playground v0.0.1 (/playground)
warning: name `III` contains a capitalized acronym
 --> src/main.rs:7:5
  |
7 |     III
  |     ^^^ help: consider making the acronym lowercase, except the initial letter (notice the capitalization): `Iii`
  |
  = note: `#[warn(clippy::upper_case_acronyms)]` on by default
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#upper_case_acronyms

warning: unneeded `return` statement
  --> src/main.rs:24:22
   |
24 |             "1" => { return Ok(MyEnum::I); },
   |                      ^^^^^^^^^^^^^^^^^^^^^ help: remove `return`: `Ok(MyEnum::I)`
   |
   = note: `#[warn(clippy::needless_return)]` on by default
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_return

warning: unneeded `return` statement
  --> src/main.rs:25:22
   |
25 |             "2" => { return Ok(MyEnum::II); }
   |                      ^^^^^^^^^^^^^^^^^^^^^^ help: remove `return`: `Ok(MyEnum::II)`
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_return

warning: unneeded `return` statement
  --> src/main.rs:26:22
   |
26 |             "3" => { return Ok(MyEnum::III); }
   |                      ^^^^^^^^^^^^^^^^^^^^^^^ help: remove `return`: `Ok(MyEnum::III)`
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_return

warning: unneeded `return` statement
  --> src/main.rs:27:20
   |
27 |             _ => { return Err("DataObject is invalid"); }
   |                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove `return`: `Err("DataObject is invalid")`
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_return

warning: length comparison to zero
  --> src/main.rs:17:12
   |
17 |         if data_object.len() == 0
   |            ^^^^^^^^^^^^^^^^^^^^^^ help: using `is_empty` is clearer and more explicit: `data_object.is_empty()`
   |
   = note: `#[warn(clippy::len_zero)]` on by default
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#len_zero

warning: writing `&Vec<_>` instead of `&[_]` involves one more reference and cannot be used with non-Vec-based slices
  --> src/main.rs:32:26
   |
32 | fn map_to_enums(strings: &Vec<&str>) -> Result<Vec<MyEnum>, String>
   |                          ^^^^^^^^^^ help: change this to: `&[&str]`
   |
   = note: `#[warn(clippy::ptr_arg)]` on by default
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#ptr_arg

warning: unneeded `return` statement
  --> src/main.rs:40:5
   |
40 |     return Ok(enums_vec);
   |     ^^^^^^^^^^^^^^^^^^^^^ help: remove `return`: `Ok(enums_vec)`
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_return

warning: returning the result of a `let` binding from a block
  --> src/main.rs:37:13
   |
35 |             let e = MyEnum::try_from(&s.to_string()).unwrap();
   |             -------------------------------------------------- unnecessary `let` binding
36 |             //TODO: if Error => return Error
37 |             e
   |             ^
   |
   = note: `#[warn(clippy::let_and_return)]` on by default
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#let_and_return
help: return the expression directly
   |
35 ~             
36 |             //TODO: if Error => return Error
37 ~             MyEnum::try_from(&s.to_string()).unwrap()
   |

warning: `playground` (bin "playground") generated 9 warnings
    Finished dev [unoptimized + debuginfo] target(s) in 2.80s

Actually, clippy doesn’t seem to address the use of &String here, but it does so in other contexts.

---

Here’s what the code looks like after adapting it a bit w.r.t. clippy suggestions and other things that came to mind. In particular, the additional case for data_object.len() == 0 seemed unnecessary, because it’s already covered by the _ => in the match.

Rust Playground