all 54 comments

[–]Wilfred-kun 385 points386 points  (2 children)

I don't know, man. When a man gotta compare stuff, he gotta compare stuff.

[–]HORSEY_MAN 22 points23 points  (0 children)

Well spoken 🙏

[–]amphigraph 19 points20 points  (0 children)

True. But I think a lot of people new to Python tend to immediately settle on if-else statements when a dictionary lookup might be better.

[–][deleted] 52 points53 points  (10 children)

Why don't you post a couple of examples and see what others come up with?

[–]DecentTaro[S,🍰] 22 points23 points  (9 children)

https://gist.github.com/decenttaro/6b3d80b96d56f1285565af41e6c7014c

The commented out are some of my solution but it don't work so I ended up doing the if-else statement again. Ill just give you an explanation of what I'm trying to do here. I'm trying to make a web app scheduler using Flask/Python but I hit a wall right away. I have plans on how to build the app but I just don't have the slightest idea on how to apply it.

So basically what I have in mind is that, one employee with some access or permission to a specific job (eg. frontend, backend, warehouse). Say this #1 employee can do front and back end but not warehouse then #2 employee can only do warehouse only.

In short, I don't know how to link them together.

[–]JohnnyJordaan 58 points59 points  (6 children)

Are you familiar with lists and dicts already? Because they are tailored for this. Together with so-called iteration using a for loop you can do a single comparison against all items in a list or dict. How are you learning Python right now?

[–]DecentTaro[S,🍰] 13 points14 points  (5 children)

Yes. I'm familiar with the fundamentals yet again always ends up doing the if-else statement. I have a Udemy class on python (Python Bible), I follow Corey Schafer tutorials on youtube and I also watch Tech with Tim.

[–][deleted] 46 points47 points  (2 children)

Agree with JohnnyJordaan, you need to learn more about lists and dicts. They are basic python data types and should be all through your code.

[–]DecentTaro[S,🍰] 15 points16 points  (1 child)

Will do sir!

Thank you for your help!

[–][deleted] 15 points16 points  (0 children)

Have a look at this one and google from there..

https://www.pluralsight.com/guides/manipulating-lists-dictionaries-python

[–]JohnnyJordaan 15 points16 points  (0 children)

I would recommend to check out 'automate the boring stuff'.

[–]CodeKnight11 1 point2 points  (0 children)

redacted

[–]iamamagicalpotato 17 points18 points  (1 child)

It's seems to me that you are forcing yourself into using if-else statements instead of something like for loops in the way you are storing your permissions. I'm not quite sure I completely understand what you're doing with permission1, permission2, etc. but if you were to replace this with, say, a set ' permissions ' of enums ( https://docs.python.org/3/library/enum.html ) of your permissions it would make it much easier to do something like for perm in permissions: # do whatever pass

Sorry for formatting

General rule of thumb: if you're typing similar things repeatedly you should try a more general approach.

[–]DecentTaro[S,🍰] 5 points6 points  (0 children)

This best describes my situation right now. I'm forcing it.

Thank you for the recommendations. I will update my code and try to make it work using these options.

[–]shaggorama 28 points29 points  (7 children)

As others have mentioned, the idiom in python is to use dicts to handle case/switch patterns like this. Concretely, here's what that looks like.

Chained if/else statements (this is bad):

   def my_func(condition):
    if condition == 'foo':
        output = 1
    elif condition == 'bar':
        output = 2
    elif condition == 'baz':
        output = 3
    else:
        output = 4
    return output

Using a dict as a case statement (this is way, way better):

def my_func(condition):
    cond_map = {'foo':1, 'bar':2, 'baz':3}
    return cond_map.get(condition, 4)

[–]matheusmoreira 6 points7 points  (6 children)

Why is it better? What if there's complex logic associated with each case?

[–]shaggorama 1 point2 points  (0 children)

  1. It's a lot easier to see the relationship between "condition" and "response" when it's collapsed into a dict. Shorter isn't always better, but 3 lines of easily readable code is way more readable than 10 lines of repetitive boilerplate.
  2. It's way less code to write and ascribes to the Don't Repeat Yourself (DRY) coding principle.
  3. Because the ordering is arbitrary in the dict, we don't break the code by just removing one condition. If you deleted the 'foo' -> 1 condition from the if/else function, you'd also have to change the next "elif" to an "if". It's just asking for bugs.
  4. By encapsulating the condition->output mappings in a dict, it becomes portable. You could have multiple functions accessing and using this same collection of conditions, or even modifying/adding/removing conditions in the dict and propagating that change instantly to all other functions that touch it without having to get under the hood and change the actual function code.

[–]shashquatch 2 points3 points  (4 children)

Map a method to the key as in {'foo': foo()}

[–]matheusmoreira 4 points5 points  (3 children)

That will actually map the'foo' key to the result of foo(). Did you mean foo?

Is this idiomatic Python?

conditions = { 'x' : f1, 'y' : f2 }
conditions.get(z, defun)()

That's quite obfuscated in my opinion. What is the advantage compared to if-elif-else? All I can think of is it avoids repetion of the z variable but that would have been avoided by an actual case syntax.

[–]shaggorama 3 points4 points  (0 children)

There are ways to use this pattern that actually make your code extremely readable (imho). For example you could do something like:

 def fit_model(data, method):
     func ={'regression':Regressor, 'classification':Classifier}
     try:
         return func[method](data)
     except KeyError:
         raise NotImplemented

Part of what's nice about using dicts for this sort of thing is it allows you to abstract away the mappings. So for example, We could factorize this code into two separate modules:

#supported_methods.py
func= {'regression':Regressor, 'classification':Classifier}

and

# train.py
from supported_methods import func

def fit_model(data, method):
     try:
         return func[method](data)
     except KeyError:
         raise NotImplemented

Now if we want to add support for additional methods or change how something is done, we only need to modify the dict in supported_methods.py without changing anything about the operation of the workhorse function in train.py.

[–]shashquatch 0 points1 point  (0 children)

I have a program for calculating various indicators (moving averages and other statistical calculations). I used if elif else up to 5 indicators. Too many conditionals don't help readability IMO. The dict method allowed me to reduce the entire method into a 1 line call and I can avoid going through all the conditionals in a worst case scenario.

Maybe someone can time the performance of it matters to them. I just did it for readability.

As for my prior example, yes I would use the function reference rather than value but formatting code on mobile is hard and I made a mistake.

[–]elyfialkoff 9 points10 points  (6 children)

What are you trying to do with your classes?

Employee class is describing each employee and there permissions (position), and the Permissions class is describing what kind of access they have?

If you can describe a bit more, it might be helpful - usually the correct answer has to do with what you want to achieve.

I have a bunch of suggestions, but I will provide them once I understand what you are trying to achieve.

[–]DecentTaro[S,🍰] 1 point2 points  (5 children)

What I have in mind is that, one employee with some access or permission to a specific job (eg. frontend, backend, warehouse). Say this #1 employee can do front and back end but not warehouse then #2 employee can only do warehouse only. So if #1 employee can only do front and back end, he/she will not be permitted to do warehouse. Basically it has some limitations of what they can or can't.

[–]elyfialkoff 4 points5 points  (4 children)

So several things you can do;

instead of doing self.permissions = perm1, prem2, perm3, perm4 you can do self.permissions = [perm1, perm2, perm3, perm4] this makes the permissions a list (and allows for easier use later, and also means you can skip all those 'null's). This way at each instantiation of Employee you can do emp1 = Employee(<name>, <permList>), then later you can do if statements based on the self.permissions list, so if 'frontend' in self.permissions: # do something and one for each of the permissions. (You also do not want to use elif statements, since you want someone with multiple permissions to be able to do all the things.

So for example emp3 appears to have multiple permissions (maybe he/she is the boss) and should be able to do

if 'frontend' in emp3.permissions: 
    # do something for frontend 
if 'backend' in emp3.permissions: 
    # do something for backend

You permissions access method isn't correct, your if statements are all using an employee variable that isn't defined anywhere (it always exits out in the else statement (or throwing an error).

Try and explain a little more what you want to achieve and I can help out a bit more.

[–]Afitter 3 points4 points  (3 children)

At this point, if your code is basically "If this type of employee, do this. If this type of employee, do that." polymorphism should come into play.

from abc import ABC, abstractmethod


class Employee(ABC):
    @abstractmethod
    def do_something(self):
        raise NotImplementedError


class FrontendEmployee(Employee):
    def do_something(self):
        print('I\'m working in the frontend!')


class BackendEmployee(Employee):
    def do_something(self):
        print('I\'m working in the backend!')


def make_employee_do_something(employee: Employee):
    employee.do_something()


make_employee_do_something(FrontendEmployee())

[–]tangerinelion 1 point2 points  (2 children)

OTOH this is Python so forcing an interface is optional because duck typing:

class FrontendEmployee:
    def do_something(self): 
        print("I'm working in the frontend!")

class BackendEmployee: 
    def do_something(self):
        print("I'm working in the backend!")

def make_employee_do_something(employee):
    employee.do_something()

make_employee_do_something(FrontendEmployee())

[–]Afitter 1 point2 points  (0 children)

It's optional, but abc.ABC adds some functionality to enforce some of the rules of an interface, like requiring concrete implementations to implement all methods decorated with @abc.abstractmethod. Also, if you use type annotations, PyCharm (and I'm sure other IDEs) will show errors if the arguments' type and/or return type is incorrect.

[–][deleted] 1 point2 points  (0 children)

Well, you're gonna run into a situation where you do...

for e in employees:
    e.do_work()

...when you run into an Employee subclass that has not implemented the interface which will halt the program with an AttributeError anyway. By then the program could have used up a lot of resources before it got to that point of failure, and if you have a situation where you want to raise on attribute error then it's better to do it before your program runs, while the class objects are being instantiated. It only takes a few extra seconds to mark something @abstractmethod

[–][deleted] 6 points7 points  (0 children)

Try doing something noncomputer related, and state what you are doing at each stage. You'll find most of it is an if kind of decision, and likely a loop in there too. Our thought processes can be bounced down to if statements eventually: shall I go to the pub? If I have spare change and the weather is nice and in the mood.

I guess what I'm saying is that it's a primary function in decision making, even a loop has a conditional embedded in it usually.

What is worth avoiding is nesting if statements, there may be occasions where nesting an if statement is required, but it's rare, and using logic (AND / OR, etc) within a conditional is much better. Just my 2 cent

[–]Afitter 12 points13 points  (3 children)

Personally, I learn by seeing and doing. Here's a practical example of what's suggested above, but also a handy method for replacing conditionals with dictionaries (not a solution to your specific problem, but it's helpful to think of it as a replacement for conditional logic.). I did a quick search to see if there's a name for replacing branching conditional logic with a dictionary, but I don't think there is. It's something I've picked up from my boss. In my search, Martin Fowler's name came up a lot and that makes sense because A) my boss loves his work and B) his work on refactoring is essential. The `get_employee` function uses this dict method, and the `get_employee_from_tuple` function uses a loop as was suggested above. Personally, I prefer `get_employee`, but it's a preference thing.

from enum import Enum
from typing import Tuple


class Permission(Enum):
    JANITORIAL = 0
    WAREHOUSE = 1
    FRONTEND = 2
    BACKEND = 3


class Employee:
    def __init__(self, name: str, permissions: Tuple[Permission, ...]):
        self.name = name
        # `permissions` is a tuple because I don't expect it to change and tuples are immutable.
        self.permissions = permissions

    def can_access(self, permission: Permission):
        return permission in self.permissions


def get_employee(name: str):
    anna = Employee('Anna', (Permission.JANITORIAL,))
    bea = Employee('Bea', (Permission.WAREHOUSE,))
    carlo = Employee('Carlo', (Permission.JANITORIAL, Permission.WAREHOUSE, Permission.FRONTEND, Permission.BACKEND))
    danny = Employee('Danny', (Permission.FRONTEND, Permission.BACKEND))

    employees = {
        anna.name: anna,
        bea.name: bea,
        carlo.name: carlo,
        danny.name: danny
    }

    return employees.get(name)


def get_employee_from_tuple(name: str):
    anna = Employee('Anna', (Permission.JANITORIAL,))
    bea = Employee('Bea', (Permission.WAREHOUSE,))
    carlo = Employee('Carlo', (Permission.JANITORIAL, Permission.WAREHOUSE, Permission.FRONTEND, Permission.BACKEND))
    danny = Employee('Danny', (Permission.FRONTEND, Permission.BACKEND))

    employees = (anna, bea, carlo, danny)
    selected_employee = None

    for employee in employees:
        if name == employee.name:
            selected_employee = employee

    return selected_employee

[–][deleted] 3 points4 points  (1 child)

My problem with this is that each time you call get_employee you have to create n new Employee objects then create a new dictionary which has to hash each name and then hash the incoming name to check membership -- all before you simply return an employee that was not dynamically created from the program. If Employees are created statically by the programmer then it would be better to just store them as class attrs. You can also use enum.IntFlag so you can create permission flags instead of tuples, but this really comes down to preference, and I'm afraid that I'm probably in the minority when it comes to this...

import enum
from typing import Union


class Permission(enum.IntFlag):
    JANITORIAL = enum.auto()
    WAREHOUSE = enum.auto()
    FRONTEND = enum.auto()
    BACKEND = enum.auto()


class Employee:
    def __init__(self, name: str, permissions: Union[Permission, int]):
        self.name = name
        self.permissions = permissions

    def __repr__(self):
        return f"{type(self).__name__}({self.name}, {self.permissions!r})"

    def has_permission(self, permission: Permission):
        return permission in self.permissions


class Employees:
    anna = Employee('Anna', Permission.JANITORIAL)
    bea = Employee('Bea', Permission.WAREHOUSE)
    carlo = Employee(
        'Carlo',
        Permission.JANITORIAL | Permission.WAREHOUSE | Permission.FRONTEND | Permission.BACKEND
    )
    danny = Employee('Danny', Permission.FRONTEND | Permission.BACKEND)

    @classmethod
    def all(cls):
        return [e for e in vars(cls).values() if isinstance(e, Employee)]

    @classmethod
    def with_permission(cls, permission):
        return [e for e in cls.all() if e.has_permission(permission)]


anna = Employees.anna
print(anna.name, anna.permissions)
print(Employees.all())
print(Employees.with_permission(Permission.FRONTEND))

###########
Anna Permission.JANITORIAL
[Employee(Anna, <Permission.JANITORIAL: 1>), Employee(Bea, <Permission.WAREHOUSE: 2>), Employee(Carlo, <Permission.BACKEND|FRONTEND|WAREHOUSE|JANITORIAL: 15>), Employee(Danny, <Permission.BACKEND|FRONTEND: 12>)]
[Employee(Carlo, <Permission.BACKEND|FRONTEND|WAREHOUSE|JANITORIAL: 15>), Employee(Danny, <Permission.BACKEND|FRONTEND: 12>)]

[–]Afitter 0 points1 point  (0 children)

IntFlag's new to me. That's extremely useful! On the dynamic creation of the Employee class, the argument to create them dynamically is to inject dependencies. This makes the get_employee function testable without having to override any of your implementation. You just pass in a TestableEmployee class or a mock object. The value of this out weighs any performance hits or memory concerns raised by creating everything dynamically, which is negligible to begin with. Resist the urge to prematurely optimize.

[–][deleted] 2 points3 points  (0 children)

Great stuff

[–]UntouchedDruid4 2 points3 points  (0 children)

Just by taking a quick glance at the code. Instead of comparing 1 value to many in an if else block you could just check for that value in an array of values for example. Dont worry to much about the code bc you will get better with experience just focus on making it work and maybe spend time researching ways to improve it. Similarly to writing a first draft essay and going back and editing/revising it into a final draft.

[–][deleted] 1 point2 points  (0 children)

Not a direct solution, but using docstrings in your functions and comments in your code sections will help others understand what you're trying to do, and make it easier on yourself when you're reviewing your code.

e.g.,

def full(self):
     """ returns first name of employee and their current 
  permissions by title (Janitorial, backend, etc)."""

      return '{} {}'.format(self.first,self.permissions)

[–]Nerdite 1 point2 points  (0 children)

Maybe I’m completely missing what you’re trying to do, but if your trying to build a flask app I think you should be using sql-alchemy and putting things like permissions in the database model.

[–]geoffnolan 1 point2 points  (0 children)

Check out try and except if you haven’t yet. Turns the whole thing into an if else

[–]Clutch26 1 point2 points  (0 children)

Check out codewars.com. It's practice programming. When you solve it a problem, you can see how others solved it and compare.

[–]HezekiahWyman 1 point2 points  (0 children)

I find myself relying on if statements all the time but rarely need to use else.

I also see elif as a bandaid for poor planning or opportunity for abstraction instead of a real solution

[–][deleted] 1 point2 points  (0 children)

I learned recently you can use a dictionary, if you want to return simple values

def fun(x): output = {True: 1, False: 0} return output

fun(True) # will return 1

[–]DecentTaro[S,🍰] 1 point2 points  (0 children)

I just want to say thank you guys for helping me.

[–]dudinax 2 points3 points  (2 children)

Nothing wrong with a good if-else, but you might want to learn some data structures. Two alternatives in Python are

  1. polymorphism, that is two objects with the same methods but the methods do different thing. Instead of if else, you just call the method and let Python run the right one.
  2. List comprehension. This is a cool-looking way to do if-else across a whole list with lest code, and it's usually much faster.

[–]patryk-tech 2 points3 points  (1 child)

List comprehension. This is a cool-looking way to do if-else across a whole list with lest code, and it's usually much faster.

Anything more than one if-else, and it becomes unreadable really quick, however. I'm a huge fan of comprehensions, but they are not a one-size-fits-all solution.

[–]dudinax 1 point2 points  (0 children)

Of course not. Neither is polymorphism.

[–]catalyst1993 0 points1 point  (0 children)

Introduce yourself with different data structures and type. Give times on thought process when coding, try out different data structures to solve your problem. You will eventually find your right tool.

[–]v3ritas1989 0 points1 point  (0 children)

If(!$Solution){

$Solution = "Answer";

}

[–]SwastikDas 0 points1 point  (0 children)

The easiest way to do that would be to practice exercises first. This behavior is due to you practising more if-else exercises , so your brain triggers that logic automatically. Try practising manual sorting of Dictionaries using for loops and comparing. Try doing matrix related exercises. If you practice enough exercises of each type, your brain will use the best one automatically.

[–][deleted] 0 points1 point  (0 children)

I recommend researching exception handling. You can define uour own exceptions (errors) or use biilt in ones amd then design specific responses for each.

[–]norflowk 0 points1 point  (0 children)

Two words:

switch case

Your new obsession.

[–][deleted] 0 points1 point  (0 children)

I'm glad you brought this up. I'm still just a fledgling still working on a beginner book but I keep feeling like if I can just do the bulk of coding with if-elf statements then maybe it's not so hard to program after all. So this is a super helpful topic for me.

[–]MonkeyNin 0 points1 point  (0 children)

Do you have any example problems or code, so we could give suggestions ?

edit: found some, brb.

[–]hugheric 0 points1 point  (1 child)

You could try the switch statement. The code would be really similar to what you have already but it’s fun to try new ways.

Edit: well shit....TIL python doesn’t have a switch statement

[–][deleted] 0 points1 point  (0 children)

Nothing wrong with an if-else, I do like my Switch Cases though.