9
\$\begingroup\$

I was doing a simple problem on codingbat and testing out my python. The problem was:

Given two int values, return their sum. Unless the two values are the same, then return double their sum.

sum_double(1, 2) → 3
sum_double(3, 2) → 5
sum_double(2, 2) → 8

My solution was:

def sum_double(a, b):
  if a is b:
    return 2 * (a + b)
  else:
    return a+b

and it worked just fine. However, when I checked the solution the answer they had was:

def sum_double(a, b):
  # Store the sum in a local variable
  sum = a + b

  # Double it if a and b are the same
  if a == b:
    sum = sum * 2
  return sum

But this seems to Java-y to me. Am I right to believe that my code is more pythonic? Or was their answer a better way to write code in python?

\$\endgroup\$
  • 19
    \$\begingroup\$ "Too Java-y"? In Java I'd write: return a==b?a*4:a+b;... \$\endgroup\$ – Olivier Grégoire Sep 27 '16 at 10:28
  • 3
    \$\begingroup\$ Yeah, I wouldn't call that solution "java-y". My job is practically all java and I would have written down your solution. I imagine that codingbat is just trying to separate the steps to make it easier to read or something? Not that it is actually easier to read unless you've got issues with basic algebra. \$\endgroup\$ – BooleanCheese Sep 27 '16 at 18:13
  • \$\begingroup\$ Would return 4 * a be simpler than return 2 * (a + b)? \$\endgroup\$ – TRiG Sep 28 '16 at 15:02
  • \$\begingroup\$ @TRiG probably yes since it can just do a << 2 but then again I wouldn't reason about it because since you branched already the interpreter may as well do the same thing for both cases. \$\endgroup\$ – noob Sep 29 '16 at 22:56
21
\$\begingroup\$

The existing answers mostly have just said is is wrong here. Which it undoubtedly is. The reason it's bad to use here is if you pass 257 and above, or -6 and below, you may get incorrect output. The best way to show how it's bad is to ask you what the output of the following are:

sum_double(2, 2)
sum_double(1 + 1, 2)
sum_double(257, 257)
sum_double(256 + 1, 257)

Simple! Surely it's 8, 8, 1028 and 1028, right? No, it's actually, 8, 8, 1028 and 514.
This is known and expected behaviour:

The current implementation keeps an array of integer objects for all integers between -5 and 256, when you create an int in that range you actually just get back a reference to the existing object.

Other than that both are fine, and I wouldn't really fret over which is 'better', this is as both are clear and understandable. Which is all your code needs to be.
A function that would be un-pythonic, as it's not clear could be:

def sum_double(a, b):
    return (a + b) * ((a == b) + 1)
\$\endgroup\$
7
\$\begingroup\$

Well, if I'd have to choose:

  • your way of comparing two integers is wrong (a is b). This is unusual and it should NOT become a habit. Just use a == b.
  • their solution also use a variable (sum) which in my opining is useless. I'd just return the needed calculus.

I'd do it this way:

def sum_double(a, b):
    return a + b if a != b else 2 * (a + b)

This is for me a pythonic way of returning the sum of two numbers using an inline return statement.

\$\endgroup\$
  • \$\begingroup\$ Even more pythonic would be respecting PEP8 (spaces between operators), SCNR ;) \$\endgroup\$ – sphere Sep 27 '16 at 6:00
  • \$\begingroup\$ @sphere I was just editing the answer. Thanks :) \$\endgroup\$ – Grajdeanu Alex. Sep 27 '16 at 6:01
  • \$\begingroup\$ What do you mean by "you are calculating the sum twice"? Isn't that the same for your proposed solution? And is it bad? \$\endgroup\$ – 200_success Sep 27 '16 at 6:14
  • \$\begingroup\$ @200_success you're right. I just had something else in mind. Fixed \$\endgroup\$ – Grajdeanu Alex. Sep 27 '16 at 6:42
  • 5
    \$\begingroup\$ If you want to avoid writing the sum again, just return 4*a if a == b. As an aside, I'm of the opinion that Python's x if y else z structure is hard to read and should be destroyed. \$\endgroup\$ – ilkkachu Sep 27 '16 at 12:53
7
\$\begingroup\$

First, the is operator is for identity comparison, the == is for equality comparison. So the a == b part is more explicit (which is more pythonic).

Also, in your solution you do the sum twice, this is against DRY.

In the second code, it is bad that they name a variable after a built-in function sum. This should never be done (unless it is intended). In this scope the built-in function sum is overridden and can't be used.

\$\endgroup\$
3
\$\begingroup\$

One of the most important things is readability.

def sum_double(a, b):
  if a == b:
    return 2 * (a + b)
  else:
    return a+b

This is the best way to write your program in this context. Use of a variable is unnecessary, and although their solution gets rid of the redundant (a+b), their code is further from the intuition that comes from the specification.

The ultimate goal is to get that perfect mixture of brief, intuitive, clear, and efficient. It's a lot to juggle but luckily efficiency isn't very relevant here.

I saw some mention of inline statements. Don't get me wrong, I like inline statements as much as the next guy, but since this function is part of the "main idea" of the program it's better to write it out and keep it clear.

If the function is not that important and is clarified in naming/comments/documentation then you could probably get away with prioritizing the inherit desire that many python programmers have to golf over readability.

\$\endgroup\$
  • 1
    \$\begingroup\$ I would find it unusual to keep the else clause when there's a return in the if statement; typically you'd write this over three lines (which, to me, also makes it easier to parse). \$\endgroup\$ – sapi Sep 28 '16 at 9:24
1
\$\begingroup\$

Aside from what others have said, I think it's better style not to use an else in cases like this, and just count on the fact that at the end of the function should be a return statement:

def sum_double(a, b):

    total = a + b
    if a == b:
        return 2 * total
    return total

This is effectively the same, but it's clearer at first glance to see that something is always returned.

The solution may consider it more Pythonic to have one return statement, but it can be better to have separate ones as you showed. Especially if the logic can become more complex, what if you had to return 0 if one of the numbers was zero, or if there's further calculations to do when the numbers are not equal? For this reason, having 2 returns (as you did) makes code neater.

Also, please use 4 spaces for indentation. It's the recommended amount and easier to read.

\$\endgroup\$
  • \$\begingroup\$ Two return points, unnecessary? How about total *= 2 instead? \$\endgroup\$ – innisfree Sep 27 '16 at 12:43
  • \$\begingroup\$ @innisfree That's slower. \$\endgroup\$ – mbomb007 Sep 27 '16 at 13:36
  • \$\begingroup\$ @mbomb007 That's about the least important of all possible aspects to this question (find one real world application where you'll notice the performance difference between the two approaches). And this is already an absolute bikeshed question. \$\endgroup\$ – Voo Sep 27 '16 at 19:58
  • \$\begingroup\$ @Voo As if having 2 returns is a problem? \$\endgroup\$ – mbomb007 Sep 27 '16 at 20:00
  • \$\begingroup\$ @mbomb007 Not sure where you get the idea that I have a problem with that. But using nano second optimisations in python code as an argument about what to prefer is rather misguided. As I said it's a bike shed problem - all shown solutions are trivial to read and just fine. \$\endgroup\$ – Voo Sep 27 '16 at 20:03
0
\$\begingroup\$

I would probably do it this way. I find that it conveys the idea most clearly while keeping it short and readable. (Flat is better than nested.)

def sum_double(aa, bb):
    mod = 2 if aa==bb else 1
    return mod * (aa + bb)
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.