Help understanding "never used" warning

code review
1

My question centers around the following snippet of code:

        let mut usebank = Bank::new();

        match file_read_file_to_string(&usepath) {
            Ok(contents) => {
                usebank = serde_json::from_str(&contents).unwrap();
            }
            Err(err) => {
                eprintln!("\n Error reading the file: {} \n", err);
                panic!("\n Error reading the file. \n");
            }
        }

In this piece of code I first create a new Bank struct and then, using serde_json read the data from a file and store it in that new Bank struct. The compiler then complains:

warning: value assigned to `usebank` is never read
   --> src/lib.rs:228:17
    |
228 |         let mut usebank = Bank::new();
    |                 ^^^^^^^

The warning is just an annoyance that I can ignore, but it may also indicate something that needs to change in the way I write code. Can someone explain to me what this is all about? Thanks!

2

You don't need to assign it a value since every branch either initializes usebank or diverges (in this case, it's a panic):

let usebank: Bank;

match file_read_file_to_string(&usepath) {
    Ok(contents) => {
        usebank = serde_json::from_str(&contents).unwrap();
    }
    Err(err) => {
        eprintln!("\n Error reading the file: {} \n", err);
        panic!("\n Error reading the file. \n");
    }
}

Or you can write it like this:

let usebank: Bank = match file_read_file_to_string(&usepath) {
    Ok(contents) => {
        serde_json::from_str(&contents).unwrap()
    }
    Err(err) => {
        eprintln!("\n Error reading the file: {} \n", err);
        panic!("\n Error reading the file. \n");
    }
};
5 Likes
3

The warning is trying to tell you that you're not storing the data in the Bank you allocated with Bank::new. You are instead discarding that Bank and replacing it with a new one allocated by serde_json::from_str(&contents).

The minimal change to your code to avoid the warning would be something like

let mut usebank: Bank = match file_read_file_to_string(&usepath) {
    Ok(contents) => serde_json::from_str(&contents).unwrap(),
    Err(err) => {
        eprintln!("\n Error reading the file: {} \n", err);
        panic!("\n Error reading the file. \n");
    }
};

Depending on what subsequent code does with the usebank variable, you may be able to remove mut from its declaration.

You can tighten this code up with a little better error handling. Failure to read and/or parse an external data file is not a bug in your code, so it should be handled with Result instead of panics. The most basic way to handle diverse errors is by mapping them to Strings, like this:

fn load_bank(path: &str) -> Result<Bank, String> {
    let contents = file_read_file_to_string(&path)
        .map_err(|err| format!("error reading {}: {}", path, err))?;
    let bank = serde_json::from_str(contents)
        .map_err(|err| format!("error parsing {}: {}", path, err))?;
    Ok(bank)
}

You can see that this also makes the code quite a bit easier to read.

In production code you would want to use a structured error type rather than strings, but that's a topic in itself.

Side note: in the second snippet I included the pathname of the problem file, and the full details of the lower-level error, in both error messages. It is really important to do both those things always when reporting errors about files. If you don't, you're making it unnecessarily difficult to troubleshoot your program.

3 Likes
4

"Using" it means not just creating it or assigning to it, but doing something with it after it is assigned/created. If you don't use it, then you don't need it, which is what the lint is checking.

5

The value assigned to usebank is not read at any point, so that value is useless (at least within this snippet). Furthermore, there is no path through the subsequent match that leaves that value in place.

Assuming you can't refactor this to use serde_json::from_reader, I'd refactor this as:

let contents = file_read_file_to_string(&usepath)?;
let usebank = serde_json::from_str(&contents)?;

If you want a smaller refactoring, this would also work:

let usebank = match file_read_file_to_string(&usepath) {
    Ok(contents) => {
        serde_json::from_str(&contents).unwrap()
    }
    Err(err) => {
        eprintln!("\n Error reading the file: {} \n", err);
        panic!("\n Error reading the file. \n");
    }
}
1 Like
6

Thanks, guys, for your comments. You were all thoughtful, focused, and understandable. I used your advice to modify my code and the warning is gone. Admittedly, my error handling is abysmal and I appreciated your suggestions for better ways to handle that. I took some notes and will factor in those changes in the near future.

:grinning:

1 Like