0

As suggested in Chapter 13, I am trying to implement the Cacher with a HashMap. Unlike some of the other questions asked, I am trying to follow the aproach taken by the author and use Option as value in the Cacher.

struct Cacher<T>
    where T: Fn(u32) -> u32, //struct needs to know Type of closure(aka calc)
{
    calc: T, 
    value: Option<HashMap<u32, u32>> 
}

impl<T> Cacher<T>
where T: Fn(u32) -> u32,{
    fn new(calculation: T) -> Cacher<T>{
        Cacher {
            calc: calculation,
            value: None
        }
    }

    fn value(&mut self, arg: u32) -> u32 {
        match &mut self.value {
            Some(map) => {
                let v = map.entry(arg).or_insert((self.calc)(arg));
                *v
            },

            None => {
                let mut map = HashMap::new();
                let v = map.insert(arg, (self.calc)(arg)).unwrap();
                self.value = Some(map);
                v
            }
        }
    }
}

The code compiles but even running a simple:

let mut expensive_res = Cacher::new( |num| {
        println!("calculating slowly....{}", num);
    thread::sleep(Duration::from_secs(1));
    num + 100
    });
println!("{}", expensive_res.value(1));

I get an Panic when running it. thread 'main' panicked at 'called Option::unwrap() on a None value'. Any suggestions? Why is unwrap here a None? Many thanks

user4815162342
  • 141,790
  • 18
  • 296
  • 355
Anatoly Bugakov
  • 772
  • 1
  • 7
  • 18

1 Answers1

1
let mut map = HashMap::new();
let v = map.insert(arg, (self.calc)(arg)).unwrap();
self.value = Some(map);
v

I think you're confused about what HashMap::insert returns: it returns the previous value for the key, if any (which is why it's an Option).

So when you've just created an empty map and you insert into it for the first time... it returns None because there can not have been an existing value there. Therefore this codepath can only panic.

incidentally the code is overcomplicated due to the completely unnecessary Option<HashMap>:

The hash map is initially created with a capacity of 0, so it will not allocate until it is first inserted into.

So putting aside the idea that this would serve any role as a temporal optimisation, it doesn't even do so, because allocation is delayed until first insertion.

There is an other issue, on both codepaths: Rust is an eager language, which means:

let v = map.entry(arg).or_insert((self.calc)(arg));

is equivalent to:

let mut entry = map.entry(arg);
let default = (self.calc)(arg);
let v = entry.or_insert(default);

so you're running the computation even when it's already in the cache, aka instead of trading memory for CPU this implementation just wastes memory.

Masklinn
  • 34,759
  • 3
  • 38
  • 57
  • thank you very much. Indeed, looking at the documentation of insert I missed that point. And similarly, given that HashMap created with 0 capacity it does mean that there is no point in using Option in this case. And thank you for pointing out the last issue as well. I don't want to allocate/run default every time. Many thanks! – Anatoly Bugakov Jan 27 '22 at 13:59