CODEWITHSUNDEEP
rustnested-map
Broken Pipe
Broken Pipe

Reputation: 2991

How to access item in nested hashmap without multiple clones?

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?

Upvotes: 0

Views: 525

Answers (2)

Niklas Mohrin
Niklas Mohrin

Reputation: 1918

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.

Upvotes: 2

Lukas Kalbertodt
Lukas Kalbertodt

Reputation: 88856

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.

Upvotes: 3

Related Questions