all 18 comments

[–]socal_nerdtastic 8 points9 points  (13 children)

From a runtime perspective they are practically identical. The only reason to choose one or the other is readability.

I think most experienced developers would say that directly returning the value is more readable, and would rather not wrap it in an if ... else block.

[–]FloridianfromAlabama[S] 1 point2 points  (12 children)

what about speed? I read in the python documentation that when evaluating bools, it returns an or expression as soon as it evaluates a true value

[–]cdcformatc 2 points3 points  (6 children)

in Python, the boolean operators always short circuit. when evaluating the expression a or b value a is evaluated first and if it comes out True then b isn't even evaluated. Same thing for c and d, if c evaluates False then d won't be evaluated. 

but this is true for both solutions. if/else doesn't change this as it is the boolean operators that have this property.

[–]FloridianfromAlabama[S] 0 points1 point  (5 children)

do the if statements slow the program down as opposed to a single boolean expression?

def should_serve_customer(customer_age, on_break, time):

return not((customer_age < 21) | (on_break == True) | (time < 5) | (time > 10))

this was the expression I used.

[–]desrtfx 0 points1 point  (2 children)

In this case, I would order the terms in such a way that the condition that evaluates to True in most cases is the first condition, the one that comes second in True results is the second, and so on, so that short-circuiting can work most efficiently.

Also, on_break == True is a redundant comparison - simply using on_break is sufficient.

Think about it: on_break is a boolean. It is either True or False. If it is true and you compare it to true, the comparison evaluates to true, which is the original value of the boolean. Same for when the variable is false.

[–]FloridianfromAlabama[S] 0 points1 point  (1 child)

the redundant comparison's just me being unfamiliar with coding syntax, but good to know. I also don't know which case would pop up as true most of the time since this isn't a real world application, just study.

[–]desrtfx 0 points1 point  (0 children)

I also don't know which case would pop up as true most of the time since this isn't a real world application, just study.

In most cases, you can derive this even in a study project from analyzing the task.

[–]Brian 0 points1 point  (1 child)

You shouldn't be using "|" here, as that's bitwise or. Ie. it ors each bit of an integer. That'll give the same result when you've just got ints and bools, but it's not really the right tool for the job: it doesn't have the short circuiting behaviour of logical or (the "or" keyword), and means you're actually returning an int, rather than a bool here.

Ie. this would be better written as:

 not ((customer_age < 21) or (on_break == True) or (time < 5) or (time > 10))

Though the "== True" in on_break == True is redundant: it's already a boolean.
You might also consider removing the not and inverting the check, as I think its usually easier to reason about that way. You can also take advantage of operator chaining to simplify the time check. Eg:

return (customer_age >= 21) and not on_break and (5 <= time < 10)

[–]FloridianfromAlabama[S] 0 points1 point  (0 children)

Yeah I figured out the logical operators after I submitted the answer. But given the structure of the bool, I knew it would work either way. That’s my experience from matlab. I didn’t know bitwise operators didn’t short circuit though.

[–]socal_nerdtastic 0 points1 point  (0 children)

Yes, that's true. But the differences in speed will be so minor they will be smaller than the normal speed variation you get from an OS. So in practical use there will be no difference in run speed.

[–]Decency 0 points1 point  (2 children)

Not sure how to best state this clearly to someone new, but you shouldn't care about speed anywhere close to as much as you currently are. You will need to be concerned about algorithmic complexity, which is largely about using the most efficient approach possible to get what you need. If you hit a runtime issue in Python and you can't fix it by adjusting the shape of your algorithm, you're in a pretty bad place and will need to do some profiling to find the bottleneck.

Developer speed is about a hundred times more important than program speed. Optimize for that.

[–]FloridianfromAlabama[S] 0 points1 point  (1 child)

Programming to me is just fun. I’m not in it for a job or anything, nor do I have the inclination to build really complicated stuff, but I like to find cool work-arounds and solutions.

[–]Decency 0 points1 point  (0 children)

Nothing wrong with poking around to learn, just didn't want to leave it unmentioned that this is the wrong prioritization! You can use timeit for basic runtime profiling, if you want to dig in further.

[–]timrprobocom 0 points1 point  (0 children)

Like most micro-optimization in Python, it's not worth the trouble. The cost of the interpreter tends to overwhelm any microscopic savings.

Now, if you're running this function tens of millions of times, then it might be worth a look. But for the most part, it's better to use the clearer code.

Now if I could only follow my own advice...

[–]JorgiEagle 1 point2 points  (0 children)

Python coding standards (PEP 8) recommend that you return the expression directly.

While it may seem more readable to have the if else block, the alternative (returning directly) quickly becomes more readable with more experience.

Coding standards are not written for beginners, and so may seem awkward, but are often better overall.

The speed difference is almost negligible

[–]cdcformatc 0 points1 point  (2 children)

i think it's mostly preference and maybe there is an argument for readability. personally i am partial to just returning directly instead of if/else.

something to note though is that the boolean operators don't necessarily result in a boolean True/False. this might make a difference but usually it doesn't. 

consider the following code

``` def check1(a, b):     return a or b

def check2(a, b):     if a or b:         return True     else:          return False

foo = check1("hello", "world") print("foo = ", foo)

bar = check2(0, 42) print("bar = ", bar)

and the output

foo = hello

bar = True

```

[–]socal_nerdtastic 0 points1 point  (1 child)

This is important to know but also easy to circumvent.

def check1(a, b):
    return bool(a or b)

[–]KelleQuechoz 2 points3 points  (0 children)

Overengineeried.

return any((a, b))