Help to beautify the ugly code

[MOCKBA]

I have a code like below

for line in String::from_utf8_lossy(&output.stdout).lines() {
                        if let Some((key,val)) = line.split_once('=') {
                            key.strip_prefix("alias ").and_then(|alias| {
                                let mut vals = val.chars();
                                if val.len() > 2 && vals.next().unwrap() == '\'' && vals.last().unwrap() == '\'' {
                                    aliases.insert(alias.to_string(), val[1..val.len() - 1].split_whitespace().map(str::to_string).collect());
                                }
                                None::<()>
                            }).or_else(|| unsafe { env::set_var(key,val); None::<_>} );
                            
                        }
                    }

Specifically, I do not like returning None, it looks weird. However using a simple if makes the code even worse. Another thing I do not like - it's the way of trimming quotes. Any other code improvement suggestions are always welcome.

[jdahlstrom]

Could you fix the indentation, or better, put the code through rustfmt? It's pretty difficult to read right now.

[quinedot]

Perhaps

    for line in String::from_utf8_lossy(&output.stdout).lines() {
        if let Some((key, val)) = line.split_once('=') {
            let Some(alias) = key.strip_prefix("alias ") else {
                unsafe { env::set_var(key, val); }
                continue;
            };

            let Some(val) = val.strip_prefix('\'') else { continue };
            let Some(val) = val.strip_suffix('\'') else { continue };
            
            aliases.insert(
                alias.to_string(),
                val.split_whitespace().map(str::to_string).collect(),
            );
        }
    }

[nerditation]

@MOCKBA

However using a simple if makes the code even worse.

why do you think so?

	for line in String::from_utf8_lossy(&output.stdout).lines() {
		if let Some((key, val)) = line.split_once('=') {
			if let Some(key) = key.strip_prefix("alias ") {
				set_alias(&mut aliases, key, val);
			} else {
				set_env(key, val);
			}
		}
	}

Rust Playground

[MOCKBA]

@jdahlstrom

put the code through rustfmt?

I will file an improvement request for the site. I can even implement it.

[MOCKBA]

It looks much better than I initially thought, however I was trying to avoid introducing functions.

[MOCKBA]

Stripping looks quite elegant. I think both can be done in one if.

[nerditation]

@MOCKBA

however I was trying to avoid introducing functions.

unlike C, rust does support function definitions within functions, so I don't see why one wound seek to avoid small utility functions, if they help improve readability.

[MOCKBA]

@nerditation

if they help improve readability.

It's hard to argue against.

[Schard]

Since all your code is within an if Some((key, val)) block, you can directly filter_map the iterator.

Rust Playground

[MOCKBA]

Thank you everybody for the valuable review. The final code looks like:

if let Ok(output) = output {
                    for (key,val) in String::from_utf8_lossy(&output.stdout).lines()
                        .filter_map(|line| line.split_once('=')) { 
                        if let Some(alias) = key.strip_prefix("alias ") {
                            if val.len() > 2 && let Some(val) = val.strip_prefix('\'') && let Some(val) = val.strip_suffix('\'') {
                                aliases.insert(alias.to_string(), val.split_whitespace().map(str::to_string).collect());
                            }
                        } else {
                            unsafe { env::set_var(key,val); }
                        }
                    }
                }

And its work result is: