From 179f8cb16c49a5f81a2132616506a9e70be7e13a Mon Sep 17 00:00:00 2001 From: Rajiv Bakulesh Shah Date: Tue, 28 Dec 2021 23:53:50 -0800 Subject: [PATCH 1/2] Refactor code in RedisCounter methods for clarity --- pottery/counter.py | 59 +++++++++++++++++++++++++++------------------- 1 file changed, 35 insertions(+), 24 deletions(-) diff --git a/pottery/counter.py b/pottery/counter.py index d58aa03e..3d6efa3d 100644 --- a/pottery/counter.py +++ b/pottery/counter.py @@ -57,23 +57,33 @@ def _populate(self, # type: ignore sign: int = +1, **kwargs: int, ) -> None: - to_set = {} + decoded_dict = {} try: - for key, value in cast(Counter[JSONTypes], arg).items(): - to_set[key] = sign * value + decoded_items = cast(Counter[JSONTypes], arg).items() + for decoded_key, decoded_value in decoded_items: + decoded_dict[decoded_key] = sign * decoded_value except AttributeError: - for key in arg: - to_set[key] = to_set.get(key, self[key]) + sign - for key, value in kwargs.items(): - original = self[key] if to_set.get(key, 0) == 0 else to_set[key] - to_set[key] = original + sign * value - to_set = {key: self[key] + value for key, value in to_set.items()} - encoded_to_set = { - self._encode(k): self._encode(v) for k, v in to_set.items() + for decoded_key in arg: + decoded_value = decoded_dict.get(decoded_key, self[decoded_key]) + sign + decoded_dict[decoded_key] = decoded_value + + for decoded_key, decoded_value in kwargs.items(): + if decoded_dict.get(decoded_key, 0) == 0: + original = self[decoded_key] + else: + original = decoded_dict[decoded_key] + decoded_dict[decoded_key] = original + sign * decoded_value + + decoded_dict = { + key: self[key] + value for key, value in decoded_dict.items() } - if encoded_to_set: + encoded_dict = { + self._encode(key): self._encode(value) + for key, value in decoded_dict.items() + } + if encoded_dict: pipeline.multi() - pipeline.hset(self.key, mapping=encoded_to_set) # type: ignore + pipeline.hset(self.key, mapping=encoded_dict) # type: ignore # Preserve the Open-Closed Principle with name mangling. # https://youtu.be/miGolgp9xq8?t=2086 @@ -105,7 +115,7 @@ def __delitem__(self, key: JSONTypes) -> None: def __repr__(self) -> str: 'Return the string representation of the RedisCounter. O(n)' - items = self.most_common() + items = self.__most_common() pairs = (f"'{key}': {value}" for key, value in items) repr_ = ', '.join(pairs) return self.__class__.__name__ + '{' + repr_ + '}' @@ -224,16 +234,15 @@ def __iset_op(self, to_del.add(k) if to_set or to_del: pipeline.multi() - if to_set: - encoded_to_set = { - self._encode(k): self._encode(v) - for k, v in to_set.items() - } - pipeline.hset(self.key, mapping=encoded_to_set) # type: ignore - if to_del: - encoded_to_del = {self._encode(k) for k in to_del} - pipeline.hdel(self.key, *encoded_to_del) - return self + if to_set: + encoded_to_set = { + self._encode(k): self._encode(v) for k, v in to_set.items() + } + pipeline.hset(self.key, mapping=encoded_to_set) # type: ignore + if to_del: + encoded_to_del = {self._encode(k) for k in to_del} + pipeline.hdel(self.key, *encoded_to_del) + return self def __ior__(self, other: Counter[JSONTypes]) -> Counter[JSONTypes]: # type: ignore 'Same as __or__(), but in-place. O(n)' @@ -248,3 +257,5 @@ def most_common(self, ) -> List[Tuple[JSONTypes, int]]: counter = self.__to_counter() return counter.most_common(n=n) + + __most_common = most_common From b8d58f7b6b518ad5dc4d3ecc337a84a40dcdc756 Mon Sep 17 00:00:00 2001 From: Rajiv Bakulesh Shah Date: Wed, 29 Dec 2021 00:13:29 -0800 Subject: [PATCH 2/2] Place better emphasis in variable names We're less concerned with what's decoded and what's encoded. We're more interested in how to populate `RedisDict`s and `RedisCounter`s based on `args` and `kwargs`. --- pottery/counter.py | 30 ++++++++++++++---------------- pottery/dict.py | 8 ++++---- 2 files changed, 18 insertions(+), 20 deletions(-) diff --git a/pottery/counter.py b/pottery/counter.py index 3d6efa3d..27ffae79 100644 --- a/pottery/counter.py +++ b/pottery/counter.py @@ -57,29 +57,27 @@ def _populate(self, # type: ignore sign: int = +1, **kwargs: int, ) -> None: - decoded_dict = {} + dict_ = {} try: - decoded_items = cast(Counter[JSONTypes], arg).items() - for decoded_key, decoded_value in decoded_items: - decoded_dict[decoded_key] = sign * decoded_value + items = cast(Counter[JSONTypes], arg).items() + for key, value in items: + dict_[key] = sign * value except AttributeError: - for decoded_key in arg: - decoded_value = decoded_dict.get(decoded_key, self[decoded_key]) + sign - decoded_dict[decoded_key] = decoded_value + for key in arg: + value = dict_.get(key, self[key]) + dict_[key] = value + sign - for decoded_key, decoded_value in kwargs.items(): - if decoded_dict.get(decoded_key, 0) == 0: - original = self[decoded_key] + for key, value in kwargs.items(): + if dict_.get(key, 0) == 0: + original = self[key] else: - original = decoded_dict[decoded_key] - decoded_dict[decoded_key] = original + sign * decoded_value + original = dict_[key] + dict_[key] = original + sign * value - decoded_dict = { - key: self[key] + value for key, value in decoded_dict.items() - } + dict_ = {key: self[key] + value for key, value in dict_.items()} encoded_dict = { self._encode(key): self._encode(value) - for key, value in decoded_dict.items() + for key, value in dict_.items() } if encoded_dict: pipeline.multi() diff --git a/pottery/dict.py b/pottery/dict.py index 4899d44b..260f2ee9 100644 --- a/pottery/dict.py +++ b/pottery/dict.py @@ -71,12 +71,12 @@ def _populate(self, ) -> None: with contextlib.suppress(AttributeError): arg = cast(InitMap, arg).items() - decoded_items = itertools.chain(cast(InitIter, arg), kwargs.items()) + items = itertools.chain(cast(InitIter, arg), kwargs.items()) encoded_dict = {} - for decoded_key, decoded_value in decoded_items: - encoded_key = self._encode(decoded_key) - encoded_value = self._encode(decoded_value) + for key, value in items: + encoded_key = self._encode(key) + encoded_value = self._encode(value) encoded_dict[encoded_key] = encoded_value if encoded_dict: