2

In chapter 13 of the Rust book, you implement a Cacher to use memoization to demonstrate functional programming and how to speed up long-running tasks. As an extra challenge, they recommend making the Cacher allow multiple keys using a HashMap and also leveraging generics to allow more flexibility.

Try modifying Cacher to hold a hash map rather than a single value. The keys of the hash map will be the arg values that are passed in, and the values of the hash map will be the result of calling the closure on that key. Instead of looking at whether self.value directly has a Some or a None value, the value function will look up the arg in the hash map and return the value if it’s present. If it’s not present, the Cacher will call the closure and save the resulting value in the hash map associated with its arg value.

The second problem with the current Cacher implementation is that it only accepts closures that take one parameter of type u32 and return a u32. We might want to cache the results of closures that take a string slice and return usize values, for example. To fix this issue, try introducing more generic parameters to increase the flexibility of the Cacher functionality.

I was able to implement the HashMap, however when trying to replace the closure definition u32 with a generic type and use that as the signature of the HashMap, I run into an issue.

use std::collections::hash_map::Entry;
use std::collections::HashMap;
use std::thread;
use std::time::Duration;

struct Cacher<'a, T>
where
    T: Fn(&'a u32) -> &'a u32,
{
    calculation: T,
    values: HashMap<&'a u32, &'a u32>,
}

impl<'a, T> Cacher<'a, T>
where
    T: Fn(&'a u32) -> &'a u32,
{
    fn new(calculation: T) -> Cacher<'a, T> {
        Cacher {
            calculation,
            values: HashMap::new(),
        }
    }

    fn values(&mut self, arg: &'a u32) -> &'a u32 {
        match self.values.entry(arg) {
            Entry::Occupied(e) => &*e.into_mut(),
            Entry::Vacant(e) => &*e.insert(&(self.calculation)(&arg)),
        }
    }
}

fn generate_workout(intensity: u32, random_number: u32) {
    let mut expensive_result = Cacher::new(|num| {
        println!("calculating slowly...");
        thread::sleep(Duration::from_secs(2));
        &num
    });

    if intensity < 25 {
        println!("Today, do {} pushups!", expensive_result.values(&intensity));
        println!("Next, do {} situps!", expensive_result.values(&intensity));
    } else {
        if random_number == 3 {
            println!("Take a break today! Remember to stay hydrated!");
        } else {
            println!(
                "Today, run for {} minutes!",
                expensive_result.values(&intensity)
            );
        }
    }
}

fn main() {
    let simulated_user_specified_value = 10;
    let simulated_random_number = 7;

    generate_workout(simulated_user_specified_value, simulated_random_number);
}

I tried K, V generics as below and it complains with Expected one of 7 possible values here pointing to the first type definition.

use std::collections::hash_map::Entry;
use std::collections::HashMap;
use std::hash::Hash;
use std::thread;
use std::time::Duration;

struct Cacher<'a, T: 'a, K: 'a, V: 'a>
where
    T: Fn(&'a K) -> &'a V,
    K: Hash + Eq,
{
    calculation: T,
    values: HashMap<&'a K, &'a V>,
}

impl<'a, T: 'a, K: 'a, V: 'a> Cacher<'a, T: 'a, K: 'a, V: 'a>
where
    T: Fn(&'a K) -> &'a V,
    K: Hash + Eq,
{
    fn new(calculation: T) -> Cacher<'a, T: 'a, K: 'a, V: 'a> {
        Cacher {
            calculation,
            values: HashMap::new(),
        }
    }

    fn values(&mut self, arg: &'a K) -> &'a V {
        match self.values.entry(arg) {
            Entry::Occupied(e) => &*e.into_mut(),
            Entry::Vacant(e) => &*e.insert(&(self.calculation)(&arg)),
        }
    }
}

fn generate_workout(intensity: u32, random_number: u32) {
    let mut expensive_result = Cacher::new(|num| {
        println!("calculating slowly...");
        thread::sleep(Duration::from_secs(2));
        &num
    });

    if intensity < 25 {
        println!("Today, do {} pushups!", expensive_result.values(&intensity));
        println!("Next, do {} situps!", expensive_result.values(&intensity));
    } else {
        if random_number == 3 {
            println!("Take a break today! Remember to stay hydrated!");
        } else {
            println!(
                "Today, run for {} minutes!",
                expensive_result.values(&intensity)
            );
        }
    }
}

fn main() {
    let simulated_user_specified_value = 10;
    let simulated_random_number = 7;

    generate_workout(simulated_user_specified_value, simulated_random_number);
}

Results in the following error:

error: expected one of `!`, `(`, `+`, `,`, `::`, `<`, or `>`, found `:`
  --> src/main.rs:16:39
   |
16 | impl<'a, T: 'a, K: 'a, V: 'a> Cacher<T: 'a, K: 'a, V: 'a>
   |                                       ^ expected one of 7 possible tokens here

Is the only way to add 2 more generics (i.e. K, V) or is there a way to reuse a single generic? If 2 required, what am I missing above?

Is there a more idiomatic approach to solving this problem? The Rust book does not offer a solution, unfortunately.

Shepmaster
  • 388,571
  • 95
  • 1,107
  • 1,366
Mike S.
  • 4,806
  • 1
  • 33
  • 35
  • What you ask is not clear, you want fix your first snipped or your second ? – Stargateur Apr 27 '18 at 06:59
  • @Shepmaster is it really necessary to reorder imports to alphabetical in your edit? Is this the preferred practice for Rust or just your personal preference? – Mike S. Apr 27 '18 at 18:02
  • 1
    @MikeS. I copy all code here, paste it in the [Rust Playground](https://play.rust-lang.org/), run [Rustfmt](https://github.com/rust-lang-nursery/rustfmt) on it, then copy it back. That's why my revision comment says "standard Rust indentation and style" and it changes much more than just those imports. I do this so that all Q&A have a consistent style that makes it easier for everyone who will ever look at these posts to not have to adapt to everyone's individual style just to get an answer. In fact, there are some changes Rustfmt makes that I disagree with, personally. – Shepmaster Apr 27 '18 at 18:06
  • Thanks for info. That makes sense. I haven't heard of `Rustfmt` yet so will check that out. – Mike S. Apr 27 '18 at 18:08

2 Answers2

2

Your implementation does not compile because lifetime bounds have to be declared only after impl:

impl<'a, T: 'a, K: 'a, V: 'a> Cacher<'a, T, K, V>
where
    T: Fn(&'a K) -> &'a V,
    K: Hash + Eq,
{
    fn new(calculation: T) -> Cacher<'a, T, K, V> {
        Cacher {
            calculation,
            values: HashMap::new(),
        }
    }
}

Storing references into the HashMap implies that you have to manage lifetimes and assure that the values referenced by HashMap outlive the Cacher.

Another approach to consider may be to cache by values:

struct Cacher<T, K, V>
where
    T: Fn(K) -> V,
{
    calculation: T,
    value: HashMap<K, V>,
}

impl<T, K, V> Cacher<T, K, V>
where
    T: Fn(K) -> V,
    K: Hash + Eq + Clone
{
    fn new(calculation: T) -> Cacher<T, K, V> {
        Cacher {
            calculation,
            value: HashMap::new(),
        }
    }

    fn value(& mut self, arg: K) -> &V {    
        match self.value.entry(arg.clone()) {
            Entry::Occupied(v) => v.into_mut(),
            Entry::Vacant(v) =>   v.insert((self.calculation)(arg)),
        }
    }
}

Please note that in this solution I added the constraint that K is Clone

Shepmaster
  • 388,571
  • 95
  • 1,107
  • 1,366
attdona
  • 17,196
  • 7
  • 49
  • 60
  • Thanks I will try this and report. By adding the clone I assume you avoid needing the lifetime and passing by reference. Any idea on the performance hit using clone or is that acceptable practice and negligible impact? – Mike S. Apr 27 '18 at 14:30
  • In first attempt (I tried before I believe) I get a lifetime issue with the return in the closure... ``` --> src/main.rs:40:10 | 40 | &num | ^^^ borrowed value does not live long enough 41 | }); | - `num` dropped here while still borrowed ``` I'll try omitting lifetimes and using clone... – Mike S. Apr 27 '18 at 14:33
  • Hi Mike, try [this](https://play.rust-lang.org/?gist=7e41c73b678d629c4ac408ee8914ee9c&version=stable&mode=debug) – attdona Apr 27 '18 at 14:37
  • Thanks! The `Clone` method does compile and work. The latest method works too. Accepting answer but I'd edit the top part and place your newest snippet in for others' benefit. Which method is recommended (using lifetimes and omitting clone, or simplifying syntax and using clone)? – Mike S. Apr 27 '18 at 14:43
  • 1
    Personally I prefer the solution where the hashmap owns the values because sounds more idiomatic to me, but it is my opinion. – attdona Apr 27 '18 at 14:52
  • Thanks. Wrapping head around Rust concepts and it seems most stumble with the ownership, borrow, lifetime at first. It's all making more sense so appreciate. I'll review some popular crates' source soon and hopefully that will illustrate accepted styles. ;-) – Mike S. Apr 27 '18 at 14:56
0

This question's pretty old, but I found this question while browsing online to see if my solution was "correct." I'll share it here, as it's slightly different that the accepted answer, and maintains the original signature for the value method in the book:

struct Cacher<T, K, V>
    where T: Fn(K) -> V,
          K: Eq + Hash + Copy,
          V: Copy {
    calculation: T,
    cache: HashMap<K, V>,
}

impl<T, K, V> Cacher<T, K, V>
    where T: Fn(K) -> V,
          K: Eq + Hash + Copy,
          V: Copy {
    fn new(calculation: T) -> Cacher<T, K, V> {
        Cacher {
            calculation,
            cache: HashMap::new(),
        }
    }

    fn value(&mut self, arg: K) -> V {
        match self.cache.get(&arg) {
            Some(&v) => v,
            None => {
                let v = (self.calculation)(arg);
                self.cache.insert(arg, v);
                v
            }
        }
    }
}

Here I have explicit generic parameters for the key and value of the HashMap that will cache values for Cacher. K has trait bounds for Eq, Hash, and Copy, and V only needs the Copy trait bound.