From 7f777213c56c2a00cac7fd2432945319f997949f Mon Sep 17 00:00:00 2001 From: Alessio Treglia Date: Wed, 4 Nov 2015 22:42:56 +0000 Subject: [PATCH] Fix misinterpretation of the disabled flag The old implementation was misinterpretating the disabled flag and effectively applying memoization even when explicitly disabled. The 'or' operator is a short-circuit one; namely, it evaluates the second argument if and only if the first is False. Therefore the following conditions caused unexpected side effects: - memoize.disabled = True, key not yet memoized Having disabled the memoize function wrapper, the client expects that no memoization happens. Instead the execution enters the if clause and store the value into the 'memo' dictionary - memoize.disabled = True, key memoized Having disabled the memoize function wrapper, the client expects that no memoization happens and the function will be evaluated anyway, whether or not its return value had already been stored in the 'memo' dictionary by a previous call. On the contrary, the last statement of wrapper() access the value stored by the last function execution. This commit attempts to improve the function readability too. --- thefuck/utils.py | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/thefuck/utils.py b/thefuck/utils.py index 2d0266ed..b840b2a0 100644 --- a/thefuck/utils.py +++ b/thefuck/utils.py @@ -23,11 +23,16 @@ def memoize(fn): @wraps(fn) def wrapper(*args, **kwargs): - key = pickle.dumps((args, kwargs)) - if key not in memo or memoize.disabled: - memo[key] = fn(*args, **kwargs) + if not memoize.disabled: + key = pickle.dumps((args, kwargs)) + if key not in memo: + memo[key] = fn(*args, **kwargs) + value = memo[key] + else: + # Memoize is disabled, call the function + value = fn(*args, **kwargs) - return memo[key] + return value return wrapper memoize.disabled = False