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

Use if-let chains when available to clean up some code #195

Closed
marco-c opened this issue Oct 8, 2018 · 3 comments
Closed

Use if-let chains when available to clean up some code #195

marco-c opened this issue Oct 8, 2018 · 3 comments

Comments

@marco-c
Copy link
Collaborator

marco-c commented Oct 8, 2018

rust-lang/rust#53667

@ghost
Copy link

ghost commented Jul 14, 2019

Hey @marco-c, curious about any performance or stylistic improvements for doing this. My understanding of if-let's are primarily for handling a single match pattern and ignoring the rest. As I look through the codebase, most comparisons seem to have two match patterns, e.g. in lib.rs:

match result.lines.entry(line_no) {
       btree_map::Entry::Occupied(c) => {
           *c.into_mut() += execution_count;
       }
       btree_map::Entry::Vacant(v) => {
           v.insert(execution_count);
       }
};

I could change that to and if-let like so:

if let btree_map::Entry::Occupied(c) = result.lines.entry(line_no) {
       *c.into_mut() += execution_count;
}
if let btree_map::Entry::Vacant(v) = result.lines.entry(line_no) {
       v.insert(execution_count);
}

Is this just the preferred syntax now, or are there performance improvements? Happy to start contributing to this project with this (feel free to assign to me), just wondering to get feedback on any technical improvements I don't know about or if this was just stylistic preference.

Cheers!

@marco-c
Copy link
Collaborator Author

marco-c commented Jul 15, 2019

It's just a stylistic preference, but not to apply in cases like the one you mentioned.

A good example is

match env::var("GCOV") {
, where:

    match env::var("GCOV") {
        Ok(s) => s,
        Err(_) => "gcov".to_string(),
    }

would become:

if let Ok(s) = env::var("GCOV") {
  s
} else {
  "gcov".to_string()
}

@ghost
Copy link

ghost commented Jul 15, 2019

Great, thanks for the feedback. I'll take a shot at this and open a PR shortly. Thanks!

@ghost ghost mentioned this issue Jul 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant