1

I have a memoizer function like so:

static Func<A, R> Memoize<A, R>(this Func<A, R> f)
{
    var cache = new ConcurrentDictionary<A, R>();
    return argument => cache.GetOrAdd(argument, f);
}

And I also have some recursive method

long TheRecursiveMeth (string inString) {
// recursive function that calls itself    
}

Now, in my main function, I try:

TheRecursiveMeth = TheRecursiveMeth.Memoize();

but the compiler complains

The '.' operator cannot be applied to operand of type `method group'

and

The left-hand side of an assignment must be a variable, a property or an indexer

How do I make calls to TheRecursiveMeth actually call TheRecursiveMeth.Memoize(), including the recursive calls?

Edit: I'm trying to avoid editing the definition of TheRecursiveMeth. Obviously I could just have that check a dictionary.

Edit 2: Since you're interested, I have a function that counts the number of certain palindromes of a given string. It's a little complicated to explain here, but basically something like:

long palCount(string inString) {
    if (inString.Length==1) return 1;
    else {
      count = 0;
      foreach(substring of inString) {
          // more complex logic here 
          count += palCount(subString);
      }
      return count;
    }
}

So clearly this type of thing would benefit from memoization. I avoided adding the algorithm at first because it is irrelevant and is more likely to get people giving me suggestions on that, which is beside the point.

dashnick
  • 2,020
  • 19
  • 34
  • 1
    Maybe `var memoized = Memoize(theRecursiveMeth)`? – Sumner Evans Aug 14 '17 at 18:45
  • Yeah but the recursive call isn't going to use that, right? – dashnick Aug 14 '17 at 18:46
  • 1
    I don't understand what you're asking. The error messages seem very clear to me, and have obvious reasons for not being allowed. Particularly confusing is your apparent attempt to reassign _the method name itself_, as well as a `Memoize()` implementation that seems to create a new cache on every call, thus negating any benefit of memorizing. Something like the suggestion in the first comment above would work fine (and is all you're going to be able to do), but the question is too confusing to understand how that would fit into your actual scenario. – Peter Duniho Aug 14 '17 at 18:55
  • 1
    @dashnick It's not used in the recursive function. It would only allow you to skip calling the recursive function altogether for identical input. If you want memoization in the recursive function, then you need to change the code in that function. – juharr Aug 14 '17 at 18:57
  • @juharr So you're saying there is no way to memoize a recursive function without actually editing the function? – dashnick Aug 14 '17 at 19:00
  • @dashnick Sure there is, you just edit the definition of that method so that it memorizes the recursive calls (using the appropriate memoizer delegate), just like juharr said. – Servy Aug 14 '17 at 19:02
  • In general yes, but I think if you wrote the recursive method in a purely functional way you might be able to do what you're attempting, but I'm not 100% sure. – juharr Aug 14 '17 at 19:03
  • @Servy editing the definition is precisely what i'm trying to avoid. – dashnick Aug 14 '17 at 19:03
  • @PeterDuniho: To simplify: if you had some recursive function `theFunc(a)` that you wanted to memoize, without actually editing the function, how would you do it? – dashnick Aug 14 '17 at 19:07
  • I'd suggest you attempt asking this question with an example of the recursive method you want to memoize and why. A good example would be `int Fib(int n) => n < 2 ? n : Fib(n-1) + Fib(n-2);` – juharr Aug 14 '17 at 19:17
  • Long story short, no that memoize function is not going to do what you want with your recursive method. You either have to rewrite the recursive method as a delegate so you can substitute in the memoization or just edit the function to include memoization. The code to actually change a method like that would be much more coplex than either of the other options if it's possible at all. – juharr Aug 14 '17 at 19:45

1 Answers1

4

For a fully generalizable solution, you'll have to change the way you write your functions in order to make this all work.

Ignoring the issues you had trying to instantiate your memoized functions, the main problem is that due to early binding, you cannot call your memoized function, the compiler will always bind to the original function. You need to give your function implementation a reference to the memoized version and let it use that. You could achieve that using a factory that takes both a function with the same signature as the memoized function, and the args.

public static Func<TArgs, TResult> Memoized<TArgs, TResult>(Func<Func<TArgs, TResult>, TArgs, TResult> factory, IEqualityComparer<TArgs> comparer = null)
{
    var cache = new ConcurrentDictionary<TArgs, TResult>(comparer ?? EqualityComparer<TArgs>.Default);
    TResult FunctionImpl(TArgs args) => cache.GetOrAdd(args, _ => factory(FunctionImpl, args));
    return FunctionImpl;
}

This then changes functions like:

public long Fib(long n)
{
    if (n < 2L)
        return 1L;
    return Fib(n - 1) + Fib(n - 2);
}

to this:

private Func<long, long> _FibImpl = Memoized<long, long>((fib, n) =>
{
    if (n < 2L)
        return 1L;
    return fib(n - 1) + fib(n - 2); // using `fib`, the parameter passed in
});
public long Fib(long n) => _FibImpl(n);

For kicks, here's an implementation of a memoizer interceptor that could be used with the Castle DynamicProxy.

class MemoizedAttribute : Attribute { }

class Memoizer<TArg, TResult> : IInterceptor
{
    private readonly ConcurrentDictionary<TArg, TResult> cache;
    public Memoizer(IEqualityComparer<TArg> comparer = null)
    {
        cache = new ConcurrentDictionary<TArg, TResult>(comparer ?? EqualityComparer<TArg>.Default);
    }
    public void Intercept(IInvocation invocation)
    {
        if (!IsApplicable(invocation))
        {
            invocation.Proceed();
        }
        else
        {
            invocation.ReturnValue = cache.GetOrAdd((TArg)invocation.GetArgumentValue(0),
                _ =>
                {
                    invocation.Proceed();
                    return (TResult)invocation.ReturnValue;
                }
            );
        }
    }
    private bool IsApplicable(IInvocation invocation)
    {
        var method = invocation.Method;
        var isMemoized = method.GetCustomAttribute<MemoizedAttribute>() != null;
        var parameters = method.GetParameters();
        var hasCompatibleArgType = parameters.Length == 1 && typeof(TArg).IsAssignableFrom(parameters[0].ParameterType);
        var hasCompatibleReturnType = method.ReturnType.IsAssignableFrom(typeof(TResult));
        return isMemoized && hasCompatibleArgType && hasCompatibleReturnType;
    }
}

Then just make sure your method is declared virtual and has the appropriate attribute.

public class FibCalculator
{
    [Memoized]
    public virtual long Fib(long n)
    {
        if (n < 2L)
            return 1L;
        return Fib(n - 1) + Fib(n - 2);
    }
}

var calculator = new Castle.DynamicProxy.ProxyGenerator().CreateClassProxy<FibCalculator>(new Memoizer<long, long>());
calculator.Fib(5); // 5 invocations
calculator.Fib(7); // 2 invocations
Jeff Mercado
  • 129,526
  • 32
  • 251
  • 272
  • Thanks! In Python I've done things like this with decorators. Can anything be done with attributes to help here? – dashnick Aug 14 '17 at 21:27
  • Using the vanilla framework, no as far as I know. It would involve rewriting the generated assemblies or adding compiler hooks to rewrite the functions. You could however create dynamic proxies to achieve similar results. That might be a more tenable solution. Something like [Castle Project](http://www.castleproject.org/projects/dynamicproxy/) (I haven't used this myself). You may be able to fully use that instead of this approach. – Jeff Mercado Aug 14 '17 at 21:33
  • Thanks Jeff. Can you by any chance explain why the `Memoized` function creates a new dictionary on every call? Shouldn't the cache be part of the state and not a local variable inside the function? I got the original one from SO here https://stackoverflow.com/a/20544642/3661120 but didn't undertand it then either. – dashnick Aug 14 '17 at 21:58
  • On the very first call of `Memoized()`, you're creating the dictionary and it returns a function that captures the dictionary. This effectively makes the dictionary local to that new function (one could even say private). You should not be calling `Memoized()` again. – Jeff Mercado Aug 14 '17 at 22:02