2048 merge function

Use automatic tests

In your doc-string you state:

    [2, 0, 2, 4] should return [4, 4, 0, 0]
    [0, 0, 2, 2] should return [4, 0, 0, 0]
    [2, 2, 0, 0] should return [4, 0, 0, 0]
    [2, 2, 2, 2, 2] should return [4, 4, 2, 0, 0]
    [8, 16, 16, 8] should return [8, 32, 8, 0]

But I either

• Test all this inputs manually (very boring)
• Trust you (nothing personal but code accuracy should not be based on personal subjective trust)

Instead you could have written:

>>> merge([2, 0, 2, 4])
[4, 4, 0, 0]
>>> merge([0, 0, 2, 2])
[4, 0, 0, 0]
>>> merge([2, 2, 0, 0])
[4, 0, 0, 0]
>>> merge([2, 2, 2, 2, 2])
[4, 4, 2, 0, 0]
>>> merge([8, 16, 16, 8])
[8, 32, 8, 0]

This way doctest will run all the test automatically each time the module is executed. This technique will save you much tedious manual testing in the future.

List comprehensions are your friend. They essentially collapse for loops into one line and directly assign them to a list, saving you pointless time. The syntax is like a for loop in reverse contained in list brackets, [statement for var in iterable]. You could first make slide like this:

    slide = [num for num in nums if num != 0]
    slide += [num for num in nums if num == 0]

But you could also use Python's truthiness to just say if not num which returns True if num is 0 and then if num for the opposite. When building slide again later you can mimic these exact two lines of course.

It's idiomatic to use i instead of idx for a for loop index, and it's shorter too.

Why use this line:

slide[idx+1] -= slide[idx+1]

When all that will ever be is basically:

slide[idx+1] = 0

Lastly, you can use list multiplication to avoid your final for loop:

slide += [0] * (len(nums) - len(slide))

(if the len is the same then their difference will be 0 anyway, so you don't need to check for a length discrepancy)

A couple of typical patterns to iterate over pairs of items in Python are:

for prev, next in zip(nums[:-1], nums[1:]:
   ...

and

prev = None
for next in nums:
    if prev is None:
        prev = next
        continue
    ...

In your case, I think the second fits the bill better:

def merge(nums):
    prev = None
    store = []

    for next_ in nums:
        if not next_:
            continue
        if prev is None:
            prev = next_
        elif prev == next_:
            store.append(prev + next_)
            prev = None
        else:
            store.append(prev)
            prev = next_
    if prev is not None:
        store.append(prev)
    store.extend([0] * (len(nums) - len(store)))
    return store

what does that mean?

A branch is when something else than the next line of code is executed. According to OWLTEST, you have 17 branches where it deems 12 the maximum acceptable.

Long story short, it doesn't like your style. There are many loops and many ifs in the same function. Splitting up your function will increase the readability significantly.

Another point it could complain about is the repetition.

    for num in nums:
        if num != 0:
            slide.append(num)
    for num in nums:
        if num == 0:
            slide.append(num)

You're iterating two times over nums right after each other. Have you thought about doing this in one iteration, appending all num != 0 to a first slide, appending all num == 0 to a second slide and paste those slides after each other later?

It would save you at least an iteration. Iterations are expensive.

I have to admit that I am quite confused by your code. It takes a lot of variables, cycles and if statements. Just looking at it I find very hard to understand what it does.

To practice a little bit with Python, I tried to do the same exercise and I came up with this (incorporating the answer of Caridorc):

def merge(nums):
  '''
  Takes a list as input 
  returns merged pairs with
  non zero values shifted to the left.
  test with: 
    python -m doctest 2048.py
  No output == OK!

  >>> merge([2, 0, 2, 4])
  [4, 4, 0, 0]
  >>> merge([0, 0, 2, 2])
  [4, 0, 0, 0]
  >>> merge([2, 2, 0, 0])
  [4, 0, 0, 0]
  >>> merge([2, 2, 2, 2, 2])
  [4, 4, 2, 0, 0]
  >>> merge([8, 16, 16, 8])
  [8, 32, 8, 0]
  '''
  # save the initial length of the list
  size = len(nums)
  # remove all the zeroes - looked up on stackoverflow
  nums[:] = (value for value in nums if value != 0)
  # now do the merging, use an index to run through the list
  i = 0
  while (i < len(nums)-1 ):
    if (nums[i]==nums[i+1]): # if a number is the same as the following
      nums[i] *= 2           # double it 
      del nums[i+1]          # remove the following
    i += 1
  # restore the initial list length appending zeroes
  while (len(nums) < size):
    nums.append(0)
  # done
  return nums

Here are the main differences compared to the original code:

1. No line looks similar to any other else. Lots of similar lines are normally an indication that there is a better way to get lighter and easier to read code.
2. Minimal usage of extra variables. Every time you declare new variables one has to understand what are you planning to use them for! The use of lots of supporting variables, especially if they are complex, normally has also an impact on performances.
3. Use while cycles instead of for cycles. The while is the perfect loop when you don't know your range a-priori. Its syntax is simpler than a for cycle and if you keep it simple is also very easy to read.
4. Avoid too many indentation levels. The deeper you go, the harder your code becomes to understand.

I hope this helps to keep up exercising writing better and better code!

Out of the initial two loops, we only need the first one. We really only want the non-zero values; we don't need to deal with the zeros until the end.

def merge(nums):
    slide = []  
    for num in nums:
        if num != 0:
            slide.append(num)

Next, we'll get rid of the main for loop. It's going to become a while loop; we'll handle incrementing ourselves.

    pairs = []
    idx = 0
    while idx < len(slide):
        num = slide[idx]
        if idx == len(slide)-1:
            pairs.append(num)
            break

We also don't need to check the lengths here, we'll deal with the end padding later. We also keep the break, this is how we get out of the loop.

Now the main check. We don't bother checking for zero equality any more, we already know there are no zeros in slide.

        if num == slide[idx+1]:
            pairs.append(num*2)
            idx += 1 

Notice the extra increment, that's so we skip the next number. We have the standard else clause, same as you had. And finally, we increment the counter ourselves.

        else:
                pairs.append(num) 
        idx += 1

Now, at this point, we have the right values we need, we just have to pad it with zeros on the right. Fortunately, you've got a way to do that already.

    for _ in range(len(nums) - len(pairs)):
        pairs.append(0) 
    return pairs

I just changed the variable name from slide back to pairs. Reusing a variable name in the way you originally did it at this point is bad practice, you should avoid it.

Also, we don't need the length check in the loop.

So, all together, the code looks like

def merge(nums):
    slide = [] 
    for num in nums:
        if num != 0:
            slide.append(num)

    pairs = []
    idx = 0

    while idx < len(slide):
        num = slide[idx]

        if idx == len(slide)-1:
            pairs.append(num)
            break

        if num == slide[idx+1]:
            pairs.append(num*2)
            idx += 1 

        else:
                pairs.append(num)  

        idx += 1

    for _ in range(len(nums) - len(pairs)):
        pairs.append(0) 
    return pairs

You'll notice that as the body of the function becomes smaller, it becomes easier to read. Also, reusing two names across three variables make the code difficult to follow. In addition, a bit of whitespace, to separate the "paragraphs" of the function, makes it easier to read.

Your style is not bad. Indentation is consistent, you stick to accepted python abbreviations (like idx, although simply using i is also acceptable), and you use decent variable names (besides the issue I've already mentioned).