Is this the best way to get value from Option<&mut >?

[ancheGT]

My code like:

#[derive(Serialize, Deserialize, Debug)]
pub struct Conf {
    locale() : String,
}

static mut CONFIG: Option<&mut Conf> = None;

pub fn get_locale() -> String {
    unsafe {
        match &CONFIG {
            Some(r) => {
                let l = r.locale() .to_string();
                return l;
            }
            None => {}
        }
    }

    return "".to_string();
}

It works well,I tried the way in How to cleanly use Option<&mut> multiple times? ,my code seems easier,but it in an unsafe block

[alice]

Why are you using a mutable reference in the first place?

[ancheGT]

Because I need a global variable,is there a better way to do this?

[VorfeedCanal]

@ancheGT

Because I need a global variable,is there a better way to do this?

What you are writing shouldn't exist at all thus, yes, of course. Any solution that is actually valid would be better.

once_cell is on the road to trd, it's better to use that then unreliable hack.

@ancheGT

my code seems easier,but it in an unsafe block

While unsafe block, by itself, is not a problem, the fact that your unsafe function is not marked as such is a problem.

Violation of soundness pledge wouldn't end well.

[steffahn]

Unsafe Rust is not all that easy to pull off. Judging by the amount of unidiomatic patterns in your code, you might not have the experience to actually check all the necessary preconditions for the unsafe features (in this case, mutable statics) you are using here. I guess, this might just for learning purposes, in which case, it’s fine to get your hands dirty and let things crash, but I just wanted to point out that for any practical code, you should probably avoid all (or all but the most straightforward) usages of unsafe.

As a side-effect, sticking to safe Rust gives a better developer experience, too; the “if it compiles, it works” experience in safe Rust has some truth to it, especially for small, straightforward pieces of code. Of course, safe Rust sometimes makes it harder to get things to compile in the first place^[1], but people in this forum are happy to help you out, in case you aren’t sure how to express (or avoid the need for) programming patterns in Rust that other languages that might be less strict about avoiding shared mutability, or require less user interaction with memory management.

---

As @VorfeedCanal pointed out, in case you need a initialize-once-then-read-only style access to the global CONFIG, then once_cell is the right crate to use. If you need more general mutable access, then using a Mutex is the safe and flexible approach. In many cases, global variables can also be avoided entirely; instead it could be a local variable in the main function, and code you call that needs access to them receives a &mut _ reference to them. If you tell us more about your use-case, I’ll gladly give recommendations of what might work well, and code examples.

---

1. though, solving compilation errors is usually pretty easy and pleasant if you keep in mind that the alternative in other programming languages might very well be debugging a mis-behaving program’s run-time behavior ↩︎

[steffahn]

@VorfeedCanal

@ancheGT

my code seems easier,but it in an unsafe block

While unsafe block, by itself, is not a problem, the fact that your unsafe function is not marked as such is a problem.

I mean, in isolation, this function really might not be unsafe yet, though of course it’s hard to imagine how it could soundly co-exist with any way to mutate the global variable without seizing to be thread-safe.

[alice]

@ancheGT

Because I need a global variable,is there a better way to do this?

Ignoring the whole discussion about whether global variables are bad, you shouldn't be using a reference here.

static mut CONFIG: Option<Conf> = None;

[steffahn]

@steffahn

the amount of unidiomatic patterns in your code

On that note, here’s some suggestions on writing more idiomatic code. I’ll start with this version of the code, since I don’t like dealing with the unsafe.

pub struct Conf {
    locale: String,
}

static CONFIG: Option<Conf> = None;

pub fn get_locale() -> String {
    match &CONFIG {
        Some(r) => {
            let l = r.locale.to_string();
            return l;
        }
        None => {}
    }

    return "".to_string();
}

The code if of course slightly weird then since the CONFIG is never changing, but that’s hopefully easy to ignore; my focus is the body of the get_locale() function anyways.

First: return EXPR; in the end of a function is always more idiomatically written just EXPR.

pub fn get_locale() -> String {
    match &CONFIG {
        Some(r) => {
            let l = r.locale.to_string();
            return l;
        }
        None => {}
    }

    "".to_string()
}

Then, in this case it’s possible, and also more readable, to transform

match {
    … => { …; return },
      ⁞
    … => { …; return },
    … => {}, // single non-early-returning branch
}
[some code…]

into

match {
    … => { …; return …; },
      ⁞
    … => { …; return …; },
    … => {[some code…]},
}

i.e. avoiding the step of leaving the match for one of the cases.

This is comparable to the more common transformation in other programming languages between

if … {
    …:
    return …;
}
[some code…]

and

if … {
    …:
    return …;
} else {
    [some code…]
}

---

Long story short, we get

pub fn get_locale() -> String {
    match &CONFIG {
        Some(r) => {
            let l = r.locale.to_string();
            return l;
        }
        None => {
            "".to_string()
        }
    }
}

In case you wonder how "".to_string() is still returned from the function even though it isn’t in the last line / final position: The whole function now consists of only a single expression, the match … {…} expression, and the whole match … {…} expression is being returned here. The match … {…} evaluates to the block { "".to_string() } in the None case, which evaluates to its final expression "".to_string().

Now, return l; is in the same kind of position as "".to_string() though, so anther return became irrelevant:

pub fn get_locale() -> String {
    match &CONFIG {
        Some(r) => {
            let l = r.locale.to_string();
            l
        }
        None => {
            "".to_string()
        }
    }
}

Avoiding the intermediate step of defining l and immediately using it:

pub fn get_locale() -> String {
    match &CONFIG {
        Some(r) => {
            r.locale.to_string()
        }
        None => {
            "".to_string()
        }
    }
}

Rewriting the match using the => EXPR, style arms instead of => { /* BLOCK */ …} to save some vertical space (in case you use it, rustfmt would even do this step for you)

pub fn get_locale() -> String {
    match &CONFIG {
        Some(r) => r.locale.to_string(),
        None => "".to_string(),
    }
}

---

That’s it for control flow for now. Next up, string functions. to_string on a &str is for many Rust programmers the less nice-looking option compared to .to_owned(), though that’s a matter of taste. On literals, some (e.g. the book) also like String::new("literal…"). For &String, I think most, if not all, people would prefer .clone(). And for an empty string it’s nicer to use String::new(). So we get

pub fn get_locale() -> String {
    match &CONFIG {
        Some(r) => r.locale.clone(),
        None => String::new(),
    }
}

Now this looks fine to me. But you could also use Option’s combinators/methods to use a different style if you like. There’s the map function that’s useful if most (or all) of your action happens in the Some case. You cannot just call it like CONFIG.map(…) here, because it takes an Option by-value, but transforming to Option<&Conf> first allows us to use it anyways

pub fn get_locale() -> String {
    CONFIG.as_ref().map(…)…
}

The map takes a closure handling the Some case:

pub fn get_locale() -> String {
    CONFIG.as_ref().map(|r| r.locale.clone())…
}

Afterwards, we have Option<String>. We can remove the Option layer, giving a fallback for None, with unwrap_or:

pub fn get_locale() -> String {
    CONFIG.as_ref().map(|r| r.locale.clone()).unwrap_or(String::new())
}

It’s cheap (essentially free) in the case of String::new, but often, unwrap_or_else can be useful to avoid function calls / extra work for constructing the fallback, in case the None case isn’t reached

pub fn get_locale() -> String {
    CONFIG.as_ref().map(|r| r.locale.clone()).unwrap_or_else(|| String::new())
}

we don’t need a closure, since String::new as a function can be named directly; a simplification sometimes called “eta conversion”

pub fn get_locale() -> String {
    CONFIG.as_ref().map(|r| r.locale.clone()).unwrap_or_else(String::new)
}

In case of String::new, since the empty string is also String::default(), i.e. the default value, you can even use the slightly shorter .unwrap_or_default(), giving us the final result

pub fn get_locale() -> String {
    CONFIG.as_ref().map(|r| r.locale.clone()).unwrap_or_default()
}

though rustfmt would have you re-format this as

pub fn get_locale() -> String {
    CONFIG
        .as_ref()
        .map(|r| r.locale.clone())
        .unwrap_or_default()
}

Choose yourself whether you prefer this, or the last match statement version above. (I myself might actually prefer the match version here.)

[ancheGT]

@steffahn

As @VorfeedCanal pointed out, in case you need a initialize-once-then-read-only style access to the global CONFIG, then once_cell is the right crate to use. If you need more general mutable access, then using a Mutex is the safe and flexible approach. In many cases, global variables can also be avoided entirely; instead it could be a local variable in the main function, and code you call that needs access to them receives a &mut _ reference to them. If you tell us more about your use-case, I’ll gladly give recommendations of what might work well, and code examples.

In my use-case,this CONIFG will initialize-once and modify many times.For example,user may want to change their locale,before that,the program will call get_locale() to get the current locale,then if user change it, the program should modifly CONIFG.locale immediately