3
\$\begingroup\$

I'm learning rust and doing an exercise where to parse a particular SGML document. Each line in a document can be either an opening tag "" , a closing tag "" or a scalar: " Val". I'm using a regex with named capture groups to detect and extract, and would like to return an enum afterwards.

I came up with the following code, but it feels overly verbose and un-idiomatic, so I'm seeking feedback, particularly on the extract_tag_from_line_match function

    use regex::Captures;
    use regex::Regex;

    struct LineParseError;
    enum Tag {
        Open(String),
        Close(String),
        Scalar(String, String),
    }
    fn extract_line_tag(line: &str) -> Result<Tag, LineParseError> {
        let reg: Regex = Regex::new(r"<((?P<close>/))|((?P<open>.+)>(?P<value>.+)?)").unwrap();

        let capture = reg.captures(line);
        match capture {
            None => Err(LineParseError),
            Some(cap) => Ok(extract_tag_from_line_match(cap)),
        }
    }

    fn extract_tag_from_line_match(capture: Captures) -> Tag {
        let close = capture.name("close");
        let open = capture.name("open");
        let value = capture.name("value");

        let scalar = open.zip(value);
        if (scalar.is_some()) {
            let (o, v) = scalar.unwrap();
            return Tag::Scalar(String::from(o.as_str()), String::from(v.as_str()));
        } else if close.is_some() {
            let c = close.unwrap();
            return Tag::Close(String::from(c.as_str()));
        } else {
            let v = open.unwrap();
            return Tag::Open(String::from(v.as_str()));
        }
    }
\$\endgroup\$
1
\$\begingroup\$

Hello and welcome to the Rust community.

Correctness

Are you sure the code is correct? The regex looks odd, but ok. It seems that Tag::Close does not carry any meaningful info, even though it contains a String.

Manual

Be sure to read up on if let statements. They are useful for concise and idiomatic code. Here, catch some reading material:

Nitpicks

  • I changed String::from to .to_string().
  • I believe you confused v with o in the last branch.
  • Don't use single letter variable names too often. (I didn't change this below.)
  • The double parenthesis around ?P<close>/ is unnecessary - single is enough.

Code

use regex::Captures;
use regex::Regex;

#[derive(Debug)]
struct LineParseError;
#[derive(Debug)]
enum Tag {
    Open(String),
    Close(String),
    Scalar(String, String),
}
fn extract_line_tag(line: &str) -> Result<Tag, LineParseError> {
    let reg: Regex = Regex::new(r"<((?P<close>/))|((?P<open>.+)>(?P<value>.+)?)").unwrap();

    let capture = reg.captures(line);
    match capture {
        None => Err(LineParseError),
        Some(cap) => Ok(extract_tag_from_line_match(cap)),
    }
}

fn extract_tag_from_line_match(capture: Captures) -> Tag {
    let close = capture.name("close");
    let open = capture.name("open");
    let value = capture.name("value");

    let scalar = open.zip(value);
    if let Some((o, v)) = scalar {
        return Tag::Scalar(o.as_str().to_string(), v.as_str().to_string());
    } else if let Some(c) = close {
        return Tag::Close(c.as_str().to_string());
    } else if let Some(o) = open {
        return Tag::Open(o.as_str().to_string());
    } else {
        unreachable!();
    }
}

fn main() {
    println!("{:?}", extract_line_tag("<val>"));
    println!("{:?}", extract_line_tag("</val>"));
}
\$\endgroup\$
4
  • \$\begingroup\$ Thanks! And double thanks for the super fast help. I have a question about to_string() . Being lazy, would it be idiomatic to add a trait to the regex::Match to_string() so that I wouldn't have to write out to_str().to_string() ? \$\endgroup\$
    – user38310
    Dec 4 '21 at 19:27
  • \$\begingroup\$ @user38310 Yes, I believe it would be idiomatic to do that. But don't be so lazy, keep in mind the code is fine as it is :) Happy to help. \$\endgroup\$ Dec 5 '21 at 8:47
  • \$\begingroup\$ @user38310 the double parenthesis around ?P<close>/ is unnecessary - single is enough. \$\endgroup\$ Dec 8 '21 at 17:56
  • \$\begingroup\$ Instead of zipping/unwrapping, you can match directly: ``` match (open, value, close) { (Some(o), Some(v), _) => { ... } ... and so on ``` I think one letter names are perfectly fine if they are short lived. It's all about readability in context \$\endgroup\$
    – Gnurfos
    Dec 10 '21 at 12:01

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.