CODEWITHSUNDEEP
pythonpython-3.xdictionary
Jinghui Niu
Jinghui Niu

Reputation: 1140

Is using type as dictionary keys considered good practice in Python?

I have a lookup table like this:

lookup = {
  "<class 'Minority.mixin'>": report_minority,
  "<class 'Majority.mixin'>": report_majority,
}

def report(o):
  h = lookup[str(type(o))]
  h()

It looks awkward to me as the key is precariously linked to how type() returns and represents a class in string. If one day Python changes its way to represents class types in string, all the codes like this are broken. So I want to have some advice from pros, is such type of keys considered good or bad? Thanks.

Upvotes: 0

Views: 4868

Answers (3)

Martijn Pieters
Martijn Pieters

Reputation: 1121654

The question would more be why are you doing this in the first place? What advantage do you think using strings has?

Class objects are hashable, so you can use Minority.mixin and Majority.mixin directly as keys. And when you do so, you ensure that the key is always the exact same object provided your classes are globals in their respective modules, making them singletons your program. Moreover, there is no chance of accidental confusion when you later on refactor your code to rename modules and you end up with a different type with that exact repr() output.

So unless you have a specific usecase where you can't use the class directly, you should not be using the string representations.

(And even if you were to generate classes with a factory function, using a baseclass and isinstance checks or extracting a base class from the MRO would probably be preferable).

So, for your usecase, stick to:

lookup = {
    Minority.mixin: report_minority,
    Majority.mixin: report_majority,
}

def report(o):
    h = lookup[type(o)])
    h()

Next, if you ensure that report_minority and report_majority are functions (and not methods, for example), you can use functools.singledispatch() and dispense with your mapping altogether:

from functools import singledispatch

@singledispatch
def report(o):
    raise ValueError('Unhandled type {}'.format(type(o)))

@report.register(Minority.mixin)
def report_minority(o):
    # handle a Minority instance

@report.register(Majority.mixin)
def report_majority(o):
    # handle a Majority instance

Note that this won't work with methods, as methods have to take multiple arguments for dispatch to work, as they always take self.

Single-dispatch handles subclasses much better, unlike your str-based mapping or even the direct class mapping.

Upvotes: 8

strubbly
strubbly

Reputation: 3477

Just remove the str from the lookup and from the dictionary. So

lookup = {
   Minority.mixin : report_minority,
   Majority.mixin : report_majority,
}

def report(o):
    h = lookup[type(o)]
    h()

This fixes your immediate concern and is fairly reasonable code. However, it seems like dispatching on the type is exactly what OO is for. So why not make those varying report functions methods on the o object derived from its type? Then you could just write:

o.report()

and the correct variant would be obtained from the class.

Upvotes: 1

Dorian
Dorian

Reputation: 761

Take a look at that script, I don't think it's perfect but it works and I think it's more elegant than your solution :

#!/usr/bin/env python3

class Person:
    pass

def display_person():
    print("Person !")

class Car:
    pass

def display_car():
    print("Car !")

lookup = {
    Person: display_person,
    Car: display_car
}


def report(o):
    lookup[o.__class__]()


report(Person())
report(Car())

Edit : modification for Python3

Upvotes: 0

Related Questions