trjhtr

Task:

Given an image object and an effect identifier (string), apply the effect identified by the string on the image. The library of effects can grow as the program grows.

Code 1:

def apply_fx(img, fx):
 if fx == 'sepia':
 return sepia(img)
 elif fx == 'b&w':
 return contrast(grayscale(img), 2)
 elif fx == 'faded':
 return faded(img)
 elif fx == 'hdr':
 return hdr(img)

Code 2:

effects = 
 'sepia': sepia,
 'b&w': lambda x: contrast(grayscale(x), 2),
 'faded': faded,
 'hdr': hdr


def apply_fx(img, fx):
 return effects[fx](img)

Opinion: I personally lean toward the second approach.

Note: the concern here generalizes to: dictionary of lambda functions vs if-elif in such scenarios.

Extra code: as suggested by @PeilonRayz.

sepia_filter = np.array([[.393, .769, .189],
 [.349, .686, .168],
 [.272, .534, .131]])


def sepia(img):
 sepia_img = np.dot(img, sepia_filter.T)
 return sepia_img / np.max(sepia_img)

IMO your second solution (using a dict to store callables) is exactly right.

The first solution, with a series of if/else statements is not only inefficient, it also encourages and facilitates writing unrelated code inside the apply_fx function itself. To wit:

elif fx == 'b&w':
 return contrast(grayscale(img), 2)

Once you have put something other than a simple return func(img) in the code, a future maintainer will be tempted to "just write this inline here" rather than creating a lambda or a named function to encapsulate their code:

elif fx = 'grayscale':
 from thirdpartymodule import grayscale, formatconverter

 gsh = grayscale.Handle()
 img3p = formatconverter(img)
 result = grayscale.grayscale(gsh, img3p)
 return formatconverter(img3p, format=img)

Your second alternative, on the other hand, has a much more restricted, and thereby much cleaner interface: all operations must be encapsulated as callables.

Now, they could be functions, or they could be lambdas. They could be functools.partials or dynamically loaded or web api calls to a satellite orbiting planet Nebulon. But from the maintenance point of view, "create a callable, plug it in here with a name" is as good as you're likely to get. You could even do it dynamically, in response to a configuration file or plugin module:

def img2grayscale(img):
 from thirdpartymodule import grayscale, formatconverter

 gsh = grayscale.Handle()
 img3p = formatconverter(img)
 result = grayscale.grayscale(gsh, img3p)
 return formatconverter(img3p, format=img)

effects['grayscale'] = img2grayscale

It appears to be premature optimisation problem. However; the 2nd approach will definitely be easier to maintain. If you wish, even the apply_fx can be implemented as a lambda call.

You can dig deeper into the effects of if-else nest vs. dict lookup here.

I second @Austin Hastings' answer but I'd like to propose a third approach. Depending on your needs this can be limited at will.

The fact is that your effects dictionary is almost "just a namespace", but namespaces have already an implementation in Python: they are modules. So defining all the function in a specific module should be enough to "gather them under a namespace". It gives you the ability to retrieve them by their name through the module or using a string thanks to getattr.

So the simplest apply_fx implementation could be either

import image_manipulation_library

def apply_fx(img, fx):
 effect = getattr(image_manipulation_library, fx)
 return effect(img)

if defined outside of said module; or

import sys

def apply_fx(img, fx):
 effect = getattr(sys.modules[__name__], fx)
 return effect(img)

if defined inside.

The obvious advantage of this approach is that it forces you to define, name and optionally document a function for the black and white effect:

def black_and_white(img):
 return contrast(grayscale(img), 2)

Which is good for the users of your code as they can introspect it better. However this will limit the names to value Python identifiers.

You can also still plug third party code by just importing them, and you can limit the results of the getattr call to ensure it fits your needs (check it is a callable, inspect it's arguments list to verify it only has a single parameter...)

In addition to the recommendations suggested by @Austin, you could inherit from dict to create an object to register and recall your functions. For example:

class Effects(dict):
 def register(self, name):
 def decorator(f):
 self[name] = f
 return f
 return decorator

Effects = Effects()


@Effects.register('sepia')
def sepia(img):
 sepia_img = np.dot(img, sepia_filter.T)
 return sepia_img / np.max(sepia_img)

@Effects.register('b&w')
def b_w(img):
 return contrast(grayscale(img), 2)

# and so on...

Effects['sepia'](img)

Now you don't have to register each effect in your dictionary after the function declaration and this should clean things up a little. Additionally, if you import this object, you can add functions anywhere, easily see what effects are registered, and you can add logic to the Effects class itself if you want more complex behavior (i.e. address the case where an invalid string is entered).

From the looks of it, you want to have an Effects class, that can be used in the following ways:

Effects.sepia(img)
Effects['b&w'](img)

This is as you can define all your code as full on functions, and for you to not have $O(n)$ lookup.
To do the above I'd use a metaclass that:

• 
  

  Implements a method, to allow you to look up by a different name than the function name.

  
  
  

  This is as b&w is an illegal function name in Python.

  
  



• You want to group all the functions in the class by their name, or altered name.



• You want to implement __getitem__ on a metaclass, so you can perform it on the class.

And so you can use:

class NamespaceMeta(type):
 def __new__(meta, classname, bases, class_dict):
 meta.lookup = lookup = 
 for key, value in class_dict.items():
 if key[0] == '_':
 continue

 metadata = getattr(value, 'metadata', )
 name = metadata.get('name', None)
 if name is None:
 name = key

 lookup[name] = value

 return type.__new__(meta, classname, bases, class_dict)

 def __getitem__(self, key):
 return self.lookup[key]


class Namespace(metaclass=NamespaceMeta):
 @staticmethod
 def metadata(name=None):
 def inner(fn):
 fn.metadata = 'name': name
 return fn
 return inner

Using the above allows you to make a module that hides all the complexity, and then implement the classes with the complexity abstracted away.

class Effects(Namespace):
 def sepia(img):
 return f'sepia img'

 @Namespace.metadata(name='b&w')
 def b_w(img):
 return f'b&w img'


Effects.sepia('abc') # sepia abc
Effects.b_w('abc') # b&w abc
Effects['sepia']('abc') # sepia abc
Effects['b&w']('abc') # b&w abc