3
\$\begingroup\$

Given a vector <Vec<usize>> of indices indicating where to split a string as follows:

println!("{:?}", idxs);
[2, 1, 6, 2, 5, 2, 2, 1]

And the following string (The Series column in the Bureau of Labor Statistics' data):

let mut sstr = "JTS000000000000000JOR";

I wrote a recursive function to split the string as follows:

fn split_str(cur: &str, rest: &str, idxs: Vec<usize>, mut res: Vec<String>) -> Vec<String> {
    if idxs.len() > 1 {
        //println!("cur: {} idx: {} rest: {}", cur, *idxs.first().unwrap(), rest);
        res.push(cur.to_owned());
        let (cur, rest) = rest.split_at(*idxs.first().unwrap());
        split_str(cur, rest, idxs[1..].to_vec(), res)        
    }
    else {
        res.push(cur.to_string()); 
        //println!("{}", rest);
        res[1..].to_vec()
    }
}

The method is run as follows:

let r = split_str(&sstr[0..idxs[0]], &sstr[(idxs[1]+1)..], idxs[1..].to_vec(), vec!["".to_string()]);

which correctly returns

["JT", "S", "000000", "00", "00000", "00", "JO", "R"]

How can I optimize this code? I'm used to Scala and it feels like I'm forcing Rust into a pattern it wasn't designed for, but I don't want to revert to traditional loops.

\$\endgroup\$
3
  • \$\begingroup\$ I see you edited the code slightly from the version on SO, but it still doesn't perform as advertised \$\endgroup\$
    – trent
    Sep 5, 2021 at 11:42
  • 1
    \$\begingroup\$ @trentcl: Thanks for pointing out the error and showing me Rust Playground. I have updated my code and linked to a working copy on that site. \$\endgroup\$
    – Lars Skaug
    Sep 5, 2021 at 15:50
  • 2
    \$\begingroup\$ Welcome to Code Review! Incorporating advice from an answer into the question violates the question-and-answer nature of this site. You could post improved code as a new question, as an answer, or as a link to an external site - as described in I improved my code based on the reviews. What next?. I have rolled back the edit, so the answers make sense again. \$\endgroup\$ Sep 5, 2021 at 16:03

2 Answers 2

2
\$\begingroup\$

In Rust, we tend to make use of iterators rather than recursion. Map, filter, reduce, etc. are your friends, especially if you don't like loops. Also, when working with parameters, slices tend to be preferable over Vecs (though returning is usually a concrete collection). Another thing is that we tend to use method chaining where possible (acting as a poor man's function composition).

Keeping the general method of splitting the string, I've refactored the given method to the following:

pub fn split_str(string: &str, places: &[usize]) -> Vec<String> {
    let (mut res, rem) = places
        .iter()
        .fold((Vec::<String>::new(), string), |(res, rem), place| {
            let (cur, rem) = rem.split_at(place);
            res.push(String::from(cur));
            (res, rem)
        });
    res.push(String::from(cur));
    res
}

Hope this was helpful!

\$\endgroup\$
3
  • \$\begingroup\$ Thanks! This is very helpful. As for your comment about a general preference for slices, is that due to performance or style preference or something else? \$\endgroup\$
    – Lars Skaug
    Sep 4, 2021 at 21:01
  • \$\begingroup\$ I'm getting a scope error on cur. By the time processing gets to res.push(String::from(cur));, cur is out of scope. I guess the trick is to initiate it above? \$\endgroup\$
    – Lars Skaug
    Sep 4, 2021 at 21:08
  • \$\begingroup\$ ... and rem.split_at(place); needs to be rem.split_at(*place); \$\endgroup\$
    – Lars Skaug
    Sep 4, 2021 at 21:09
0
\$\begingroup\$

I ended up rewriting the function with fold as follows:

fn str_splits(s: String, is: &[usize]) -> Vec<String> {
   let (first, mut rest) = s.split_at(is[0]);  
   let cols =  is.iter().skip(1).fold(vec![first.to_string()],  |mut v, nxt|  {
    //println!("Current vector: {:?} Next: {}, Remaining String: {}", v, nxt, rest);
    let (cur, rst) = &rest.split_at(*nxt);
    rest = rst;   
    v.push(cur.to_string());
    v});
    cols
}

Given the following slice and string

let cs = str_splits(&[2, 1, 6, 2, 5, 2, 2, 1]
let s = "JTS000000000000000JOR".to_string();

The function can be run as follows:

let r = str_splits(s, cs);

Which correctly returns

["JT", "S", "000000", "00", "00000", "00", "JO", "R"]

A demonstration can be found here.

I believe this version is easier to grasp and more in line with Rust style. Fold has also proved to be fast in other cases.

\$\endgroup\$
1
  • 1
    \$\begingroup\$ Could you perhaps explain more about why this is better than the original code? Please remember that this is code review, not code alternatives. Please start by noting where the question code can be improved. Then illustrate that improvement with code in the answer (or not, code is not actually required in an answer, although strongly recommended). \$\endgroup\$
    – mdfst13
    Sep 4, 2021 at 23:31

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.