2
\$\begingroup\$

I am learning Python. I have a scenario in my current project to benchmark the following code snippet; by using refactoring to increase the performance. Upon deployment my code will be used on a larger scale.

I'm trying from my side and am also looking for suggestions/guidance from you all. Is there a way to make this code much faster and cleaner with less line of code?

def test(x1, x2):
    if x2 is None:
        return
    elif len(x2) == 1:
        x1.append(x2)
        return
    else:
        for i in range(len(x2)):
            x1.append(x2[i])
    return

l = ['tt', 'yy', 'pp']
test(l, None)
print (l)


test(l, ['hello'])
print(l)

test(l, ['hello', 'world'])
print(l)
\$\endgroup\$
  • \$\begingroup\$ Could you attempt to rewrite your question a bit? I don't really understand what it is that you want help with. I also don't understand what the purpose of your function is. What performance are you speaking of (memory usage or speed), and how is it relevant to this function? Do you only want suggestions on how to make the algorithm faster? \$\endgroup\$ – ades Sep 25 at 13:47
  • 2
    \$\begingroup\$ Your code looks very much like an example. Please note that Code Review is the exact opposite of Stack Overflow in that regard. On Stack Overflow, you should not post your actual code, you should instead construct a minimal reproducible example. Here on Code Review, however, you should not post incomplete, hypothetical, or example code, but instead your real-world, actual, working code. \$\endgroup\$ – Jörg W Mittag Sep 25 at 19:07
  • 2
    \$\begingroup\$ Also, please explain a bit more, what does your code do and what do you want from us? Remember: in a "real world" code review, the code review would be held with a specific purpose (improving throughput, reducing memory usage, find security holes, improve readability, etc.) and you would be in the room with us, walking us through your code line-by-line explaining every choice, every variable, every function. On this website, all we have is your question, so you need to make sure that all the information that we would normally get during the code review session is part of the question. \$\endgroup\$ – Jörg W Mittag Sep 25 at 19:08
  • 2
    \$\begingroup\$ Please do not update the code in your question to incorporate feedback from answers, doing so goes against the Question + Answer style of Code Review. As such I have rolled back your latest edit. Please see what you may and may not do after receiving answers. \$\endgroup\$ – Peilonrayz Sep 26 at 11:59
5
\$\begingroup\$

Use extend() instead of append() to add a list to another list, and use the fact that None and empty lists compare as False, while a non-empty list compares as True, and omit the redundant return statement:

def test(x1, x2):
    if x2:
        x1.extend(x2)

As for reducing the number of print statements, prepare a list or tuple with your test cases, and handle them all in a single for-loop:

l = ['tt', 'yy', 'pp']
testcases = [
    None,
    ['hello'],
    ['hello', 'world']
]

for testcase in testcases:
    test(l, testcase)
    print(l)
| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ This solution does not capture the elif-logic in the original code. \$\endgroup\$ – GZ0 Sep 26 at 4:45
  • \$\begingroup\$ @GZ0 It looks like you're correct, in case of x2 being a list of one element the original code appends a list instead of just the element. I assume that was not the intention though. \$\endgroup\$ – G. Sliepen Sep 26 at 8:24
  • \$\begingroup\$ Thanks, @Sliepen. I was looking for a way to write the if-else statements using list comprehension.But didn't find anything in docs, \$\endgroup\$ – Durga Sep 26 at 10:51
  • 1
    \$\begingroup\$ I was able to reduce the line of code. But the output is slightly different from the expected one. Updating my question as per the same \$\endgroup\$ – Durga Sep 26 at 11:40
1
\$\begingroup\$

Here is my code review.

  • It is a good practice to add type hints to Python functions to specify parameter types. See here for more details. Note that type hints do not affect code execution. It is primarily for improving readability and helping IDEs look for potential bugs.

  • When running code outside a function / class, it is a good practice to put the code inside a main guard. See here for more details.

      if __name__ == "__main__":
         ...
    
  • The for-loop could be improved by using +=, which is equivalent to extend but shorter and faster.

  • If the None input for x2 happens rarely, one can use try ... except to catch the exception from len(x2) call in order to improve performance.

Following is my version of improved code. Since the types of input parameters are not explained, I will make the most general assumption that x1 is a list and x2 is a Sequence or None.

from typing import Optional, List, Sequence

def test(x1: List, x2: Optional[Sequence]):
    try:
        if len(x2) == 1:
            x1.append(x2)
        else:
            x1 += x2
    except TypeError:
        pass

if __name__ == "__main__":
    x1_input = ['tt', 'yy', 'pp']
    x2_inputs = [
        None,
        ['hello'],
        ['hello', 'world']
    ]

    for x2_input in x2_inputs:
        test(x1_input, x2_input)
        print(x1_input)

Output:

['tt', 'yy', 'pp']
['tt', 'yy', 'pp', ['hello']]
['tt', 'yy', 'pp', ['hello'], 'hello', 'world']

The function could be further shortened as follows:

def test(x1: List, x2: Optional[Sequence]):
    try:
        x1 += [x2] if len(x2) == 1 else x2
    except TypeError:
        pass
| improve this answer | |
\$\endgroup\$

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