0

In my execute method of executor class I have lots of if elif statements in order to know what specification is passed and run some code depending on the specification.

Is there a way to make the code nicer? I have nearly 10 elif statements following each other, is there something else I could use instead?

Sample of code:

class Executor:

    def execute(self, spec):
        if spec.print && spec.run:
            self._print(spec)
            self._run(spec)

        elif spec.run:
            self._run(spec)

        elif spec.print:
            self._print(spec)

        else:
            self.skip(spec)


    def _run(self, spec):
        ...
    def _print(self, spec):
        ...
    def _skip(self, spec):
        ...

Executor().execute(spec)
Russia Must Remove Putin
  • 374,368
  • 89
  • 403
  • 331
ThinkGeek
  • 4,749
  • 13
  • 44
  • 91
  • 2
    This looks wonky from the outset. `Executor().execute(spec)`? `&&`? Are you trying to use Java patterns? – roganjosh Mar 27 '20 at 11:58
  • @roganjosh The intention is to clean this code by removing these unwanted if conditions. Nothing like want to use Java patterns... – ThinkGeek Mar 27 '20 at 12:00
  • 1
    I don't mean in terms of indentation, I mean in terms of actual coding patterns. `Executor().execute(spec)` could be `Executor.execute(spec)` if you made `execute` a `@staticmethod`. But that wouldn't make much sense. What is `spec`? `&&` isn't valid in Python – roganjosh Mar 27 '20 at 12:02
  • maybe you want dictionaries. but without more context, it's hard to tell. also, i don't see why you're duplicating what needs to run in the branches, `spec.print && spec.run` are running the same steps as the two branches below it. So you could switch the logic around and eliminate one branch entirely. – Paritosh Singh Mar 27 '20 at 12:11
  • Instead of having the attributes of ``spec`` select methods of ``Executor``, why not directly select ``Executor`` methods? E.g. ``Executor._run`` or ``Executor._print`` (ideally with proper naming, i.e. without leading `_`). Note that there is also [``operator.attrgetter``](https://docs.python.org/3.4/library/operator.html#operator.attrgetter) – instead of ``spec.run`` one could use ``attrgetter('run')``. – MisterMiyagi Mar 27 '20 at 13:51
  • https://stackoverflow.com/questions/42020589/replace-many-else-if-with-something-else similar problem in Java is this helps – ThinkGeek Mar 27 '20 at 14:24

1 Answers1

1

You want "polymorphism"? I think your code can be simplified a little:

class Executor:

    def execute(self, spec):
        if spec.print:
           self._print(spec)

        if spec.run:
           self._run(spec)

        if not (spec.run or spec.print):
           self.skip(spec)


    def _run(self, spec):

    def _print(self, spec):

    def _skip(self, spec):

Executor().execute(spec)

But that's not really polymorphism.

You seem to know a lot about the spec object. You say:

I have lots of if elif statements in order to know what specification is passed

So do you have the ability to change implementation of spec or pass in a subclass of spec? It seems to know if it wants to print, run, or skip:

class Executor:

    def execute(self, spec):
        spec.print(self)
        spec.run(self)
        spec.skip(self)

Executor().execute(spec)

If you can instantiate your Executor based on the decisions made by spec you can "simplify" with subclasses:


from abc import ABC, abstractmethod

class ExecutorMixin(ABC):
    __slots__ = ()

    @abstractmethod
    def execute(self):
        raise NotImplementedError

    def _run(self, spec):
        ...

    def _print(self, spec):
        ...

    def _skip(self, spec):
        ...

ExecutorMixin is an interface that you can subclass with the specific behavior and do your operation based on the type of Executor:

class ExecutorPrintAndRun(ExecutorMixin):
    def execute(self, spec):
        self._print(spec)
        self._run(spec)

class ExecutorRun(ExecutorMixin):
    def execute(self, spec):
        self._run(spec)

class ExecutorPrint(ExecutorMixin):
    def execute(self, spec):
        self._print(spec)

class ExecutorSkip(ExecutorMixin):
    def execute(self, spec):
        self._skip(spec)

Then you instantiate the class you want to use:

ExecutorSkip().execute(spec)
Russia Must Remove Putin
  • 374,368
  • 89
  • 403
  • 331
  • Thanks for the detailed answer. My idea is to keep the client code spec agnostic by calling Executor.execute(spec). I am trying to understand if I can make it a polymorphic inside the execute method of the executor class. Do you think there is some way? – ThinkGeek Mar 27 '20 at 13:44
  • As per your code, if-else moved one level up where I would have to make a decision by opening spec class. Something like if spec.print ExecutorPrint.execute(spec) ... – ThinkGeek Mar 27 '20 at 13:47