QueryString improvements - questions

[dEajL3kA]

Hello.

After completing your Rust course, I try to improve QueryString to handle "percent"-encoded keys and values, by using the urlencoding crate. I figured I have to change QueryString and Value to store String objects instead of &str slices, so:

#[derive(Debug)]
pub struct QueryString {
    data: HashMap<String, Value>,
}

#[derive(Debug)]
pub enum Value {
    Single(String),
    Multiple(Vec<String>),
}

Unfortunately, the urlencoding::decode() functions returns Cow<str> instead of normal String or &str 🤯

How do I "convert" the Cow<str> into String? Should it even be converted? Is the following helper function reasonable ???

use urlencoding::decode;

fn url_decode(str: &str) -> String {
    match decode(str) {
        Ok(decoded) => decoded.into_owned(), // <-- the best way to get String from Cow<str> ???
        Err(_) => str.to_owned(),
    }
}

...or better to store Cow<str> instances in the Value ??? 😅

---

Another problem arises when key and val are of type String now:

impl From<&str> for QueryString {
    fn from(s: &str) -> Self {
        let mut data = HashMap::new();

        for sub_str in s.split('&').filter(|x| !x.is_empty()) {
            let mut parts = sub_str.splitn(2, '=');
            let key = parts.next().map(str::trim).map(url_decode).unwrap_or_default(); // <-- String
            let val = parts.next().map(str::trim).map(url_decode).unwrap_or_default(); // <-- String

            data.entry(key.to_owned())
                .and_modify(|existing| match existing {
                    Value::Single(prev_val) => {
                        *existing = Value::Multiple(vec![prev_val.to_owned(), val]);
                    }
                    Value::Multiple(vec) => vec.push(val),
                })
                .or_insert(Value::Single(val)); // <-- ERROR: use of moved value: `val` ☹️
        }

        QueryString { data }
    }
}

The value val gets moved into the closure (at and_modify()), even though I don't use the move keyword 😢

Is there any way to prevent that? Solution I have found is by using clone() method:

let key = parts.next().map(str::trim).map(url_decode).unwrap_or_default();
let val = parts.next().map(str::trim).map(url_decode).unwrap_or_default();

data.entry(key.to_owned())
    .and_modify(|existing| match existing {
        Value::Single(prev_val) => {
            *existing = Value::Multiple(vec![prev_val.to_owned(), val.clone()]); // <-- clone!
        }
        Value::Multiple(vec) => vec.push(val.clone()), // <-- clone!
    })
    .or_insert(Value::Single(val));

Is this a proper solution, or how do I go about this ???

It feels like there is an unnecessary clone in the and_modify() branch now. That's because if we actually enter the and_modify() branch, then the or_insert() branch will never be entered – and vice versa. But how do we solve it?

Best regards!

[dEajL3kA]

Sorry to answer my own question:

1. It seems Cow<str> is a wrapper containing either an owned String or a borrowed &str. We could call .into_owned() to convert them into owned String objects. But I figured it would be better to just store Cow<str> in the Value, because it avoids the unnecessary clones that would happen in case that the Cow<str> contains borrowed &str.

2. Inserting the value into the HashMap can be re-written like this:

   let mut parts = sub_str.splitn(2, '=');
   let key = decode(parts.next().unwrap_or_default());
   let val = decode(parts.next().unwrap_or_default());
   
   match data.entry(key) {
       Entry::Occupied(mut entry) => {
           let existing = entry.get_mut();
           match existing {
               Value::Single(prev) => {
                   *existing = Value::Multiple(vec![mem::take(prev), val]);
               }
               Value::Multiple(vec) => vec.push(val),
           };
       },
       Entry::Vacant(entry) => {
           entry.insert(Value::Single(val));
       }
   };