How to access item in nested hashmap without multiple clones?

Question

There is a TokenBalances struct defined using a nested map. I want to implement a the balance_of method which takes two keys: token, account as input, and returns the balance as output.

use std::collections::*;

type TokenId = u128;
type AccountId = u128;
type AccountBalance = u128;

#[derive(Default, Clone)]
struct TokenBalances {
    balances: HashMap<TokenId, HashMap<AccountId, AccountBalance>>,
}

impl TokenBalances {
    fn balance_of(&self, token: TokenId, account: AccountId) -> AccountBalance {
        self.balances
            .get(&token)
            .cloned()
            .unwrap_or_default()
            .get(&account)
            .cloned()
            .unwrap_or_default()
    }
}

fn main() {
    println!("{}", TokenBalances::default().balance_of(0, 0)) // 0
}

It uses cloned twice to turn an Option<&T> to Option<T>

I'm aware of to_owned as an alternative to cloned, but in its docs it says

Creates owned data from borrowed data, usually by cloning.

I'm wondering if the clones are really necessary. Is there any idomatic way to rewrite the method without cloning twice? And are clones completely avoidable?

Why is read-only references to the data bad? Why are you cloning at all? — Kevin Anderson, Oct 04 '22 at 13:24
@KevinAnderson clone is used to turn Option<&T> returned by `get()` to Option for convenience — btwiuse, Oct 04 '22 at 13:30

score 3 · Answer 1 · answered Oct 04 '22 at 13:24

You can use Option::and_then:

self.balances
    .get(&token)
    .and_then(|map| map.get(&account))
    .copied()
    .unwrap_or_default()

The and_then call returns an Option<&AccountBalance>. This is then cloned/copied with copied. This is fine as it's just a u128 right now. If it ever becomes a more complex type where copying isn't cheap, make balance_of return &AccountBalance instead. Then you can remove the copied() call and unwrap_or(&0) for example.

Last note: the unwrap_or_default might hint at a code smell. I don't know your context, but it might make more sense to actually return Option<AccountBalance> from the method instead of defaulting to 0.

score 2 · Answer 2 · answered Oct 04 '22 at 13:25

Your need to clone comes from the fact that you are using unwrap_or_default inbetween. Without the clone, you have an Option<&HashMap> (as per HashMap::get on the outer map) and &HashMap does not implement Default - where should this default reference point to? Luckily, we don't actually need an owned HashMap, the reference to the inner map totally suffices to do a lookup. To chain two functions that may return None, one uses Option::and_then. It is like Option::map, but flattens an Option<Option<T>> into just an Option<T>. In your case, the code would look like this:

impl TokenBalances {
    fn balance_of(&self, token: TokenId, account: AccountId) -> AccountBalance {
        self.balances
            .get(&token)
            .and_then(|inner_map| inner_map.get(&account))
            .copied()
            .unwrap_or_default()
    }
}

I also changed the final cloned to copied, which indicates that this clone is cheap.

How to access item in nested hashmap without multiple clones?

2 Answers2