10
\$\begingroup\$

Yes I know context managers are a much better way to do this, but I spent four hours working on implementing a switch construct using generic decorators, and I'd like to hear feedback on how I did. I'll do it with context managers tomorrow.

 
I don't think I qualify as a beginner anymore (I'm looking to start getting paid to write Python soon), and I would appreciate guidance on best practices.


My Code

"""Module to implement C style switch functionality using decorators.

  Functions:
    `switch_construct`: Returns functions that provide the required functionality and provides a closure for said functions.
"""

__all__ = ["switch", "case", "break_"]

import operator as op

def switch_construct():
  """Forms a closure to store the meta data used by the switch construct.

    Vars:
      `closure` (list): List to store dictionaries containing metadata for each `switch` instance.
        * The dictionary contains two keys: "control" and "halt".
          - "control": the value governing the current switch instance. `case` blocks are tested against this value.
          - `halt`: value is used to implement break functionality. 
            > `True`: break after execution of current function.
            > Default: `False`.

    Returns:
      tuple: A tuple of functions (`switch`, `case`, `break_`) defined in the function's scope.
  """
  closure = []

  def switch(input_):
    """Sets the metadata for the current `switch` block.

      Args:
        `input` (object): The value governing the switch construct.

      Returns:
        `function`: A decorator for the function enclosing the switch block.
    """
    nonlocal closure
    def decorator(func):
      closure.append({"control": input_, "halt": False})  # Add the metadata to closure.
      # Sets `control` to `input_` and `halt` to `False` for each `switch` block.
      def wrapper(*args, **kwargs):
        return func(*args, **kwargs)
      wrapper.switch_index = len(closure)-1 
      # Assign the `switch_index` attribute to the decorated function for using when retrieving switch construct metadata.
      return wrapper
    return decorator

  def case(value, enclosing, test = op.eq):
    """Returns a decorator which executes decorated function only if its `value` argument satisfies some test.

      Args:
        `value` (object): value used to test whether the decorated function should be executed.
        `enclosing` (function): enclosing function corresponding to the current switch block.
        `test` (function): test that should be satisfied before decorated function is executed.
          - Default value is `op.eq`.
          - Returns `bool`.

      Returns:
        function: A decorator for the function corresponding to the case block.
    """
    def decorator(func):
      nonlocal closure
      index = enclosing.switch_index
      return_value = None
      if test(value, closure[index]["control"]) and not closure[index]["halt"]:
        return_value = func()
      return return_value
    return decorator

  def break_(enclosing):
    """Used to indicate that the switch construct should break after execution of the current function."""
    nonlocal closure
    closure[enclosing.switch_index]["halt"] = True
    return None

  return case, switch, break_


case, switch, break_ = switch_construct()

@switch(2)
def func():
  """Sample use case of the switch construct."""
  @case(1, func)
  def _(num=10):
    print(num + 1)

  @case(2, func)
  def _(num=10):
    print(num + 2)
    break_(func)

  @case(3, func, op.gt)
  def _(num=10):
    print(num + 3)

func()
\$\endgroup\$
1
  • 4
    \$\begingroup\$ Interesting. I'm somewhat surprised it works, but it's definitely interesting. \$\endgroup\$
    – Mast
    Commented Nov 26, 2019 at 15:23

1 Answer 1

6
\$\begingroup\$

I wish I had enough practice with decorators to be able to comment on most of this. I've never really needed to play with them, so I've kind of neglected them beyond simple memoization toys. This looks pretty cool, but I think all I can comment on are a few things along with the low-hanging PEP fruit since no-ones jumped on that yet :)

len(closure)-1

This should have spaces in there:

Always surround these binary operators with a single space on either side: assignment (=), augmented assignment (+=, -= etc.), comparisons (==, <, >, !=, <>, <=, >=, in, not in, is, is not), Booleans (and, or, not).

It doesn't explicitly mention -, but the "No:" examples after that bit above suggest that it should use spaces as well.

And on the other end,

test = op.eq

Shouldn't use spaces (from the link above again):

Don't use spaces around the = sign when used to indicate a keyword argument, or when used to indicate a default value for an unannotated function parameter.


break_ doesn't need a return None in there. That's implicit.


Some of what you're documenting with doc-strings could be documented in other arguably cleaner ways.

For example, in switch_construct you have:

* The dictionary contains two keys: "control" and "halt".
          - "control": the value governing the current switch instance. `case` blocks are tested against this value.
          - `halt`: value is used to implement break functionality. 
            > `True`: break after execution of current function.
            > Default: `False`.

You have a dictionary that should only take on certain keys. This sounds more like a job for a dataclass instead of a dictionary:

from dataclasses import dataclass

@dataclass
class SwitchMetadata:
    """- "control": the value governing the current switch instance. `case` blocks are tested against this value.
       - `halt`: value is used to implement break functionality.
    """

    control: str
    halt: bool = False  # Makes the constructor allow halt to default to False

. . .

closure.append(SwitchMetadata(input_))

. . .

if test(value, closure[index].control) and not closure[index].halt

. . .

closure[enclosing.switch_index].halt = True

This gets rid of the need for string dictionary lookups and gives some type safety. Both of these decrease the chance you'll make silly typos since both allow for static checking by the IDE. The class definition also clearly communicates to the reader what the structure holds and what types the data should be.

\$\endgroup\$
1
  • \$\begingroup\$ Thanks a lot for the answer. I've actually read PEP 8 in its entirety, so my stylistic deviations weren't due to ignorance. I wrote test = op.eq because I found that more readable, but I would change it in the updated version of the code. PEP 8 provides no recommendation for the operators not listed there (they use both a ∆ b and a∆b as examples of how to write code ( being an arbitrary binary operator not in the above list). The dataclass recommendation was greatly appreciated, and I've refactored my code to use one. \$\endgroup\$ Commented Nov 27, 2019 at 3:16

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.