From: Principles of Computing Part 1 Course on Coursera
I got -2 pts on my OWLTEST which uses Pylint for style guide. The error states:
Too many branches (17/12) function "merge", line 7
what does that mean?
I worked really hard on making this program work. I wrote this from scratch. I would also like to know if there are some techniques to make this cleaner and/or some best practice improvements? I know there are probably ways to write this in a better way because right now my code looks really messy.
I wish to improve:
1. Code Style
2. Performance
3. Readability
# -*- coding: utf-8 -*-
"""
Created on Thu Sep 3 17:55:56 2015
2048_merge_attempt1.py
@author: Rafeh
"""
def merge(nums):
'''
Takes a list as input
returns merged pairs with
non zero values shifted to the left.
[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]
'''
slide = [] # Append non-zeroes first
for num in nums:
if num != 0:
slide.append(num)
for num in nums:
if num == 0:
slide.append(num)
pairs = []
for idx, num in enumerate(slide):
if idx == len(slide)-1:
pairs.append(num)
if len(pairs) != len(nums):
pairs.append(0)
break
if num == slide[idx+1]:
if num != 0:
pairs.append(num*2)
slide[idx+1] -= slide[idx+1]
# slide[idx+1], slide[idx+2] = slide[idx+2], slide[idx+1]
else:
pairs.append(num)
else:
pairs.append(num) # Even if they don't match you must append
slide = [] # Append non-zeroes first
for num in pairs:
if num != 0:
slide.append(num)
for num in nums:
if num == 0:
slide.append(num)
for _ in range(len(nums) - len(slide)):
if len(nums) != len(slide):
slide.append(0)
return slide
UPDATE: Thank you, everyone, for your amazing suggestions. I incorporated a few techniques from pretty much all of your suggestions! I realized there were a lot of logical mistakes and redundancy in my code. Like why append 0's over and over again when it's not necessary at all, because in the end I back-track, account for missing numbers, and then append 0(s) in their place.
Improvements in New Code vs. Old Code:
1. Use of list comprehensions
2. Use of Python's built-in truthiness
3. Minimal branches (Less if(s), else(s), etc)
4. Infusing new and useful methods like extend
into my code
5. Improving the code style by enhancing readability
6. Removing redundandcy and flawed logic
It's amazing how a puny -2 took me on this wild detour of trying to simply improve the style. However, in the process, I learned a tremendous amount of new and powerful concepts! This was all very insightful for me and I took away a lot from your guys' answers! Here is my new and improved, final, code:
def merge(nums):
'''
Takes a list as input
returns merged pairs with
non zero values shifted to the left.
fancy interactive doc test below, no output means no problems.
>>> 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]
'''
slide = [num for num in nums if num]
pairs = []
for idx, num in enumerate(slide):
if idx == len(slide)-1:
pairs.append(num)
break
elif num == slide[idx+1]:
pairs.append(num*2)
slide[idx+1] = None
else:
pairs.append(num) # Even if not pair you must append
slide = [pair for pair in pairs if pair]
slide.extend([0] * (len(nums) - len(slide)))
return slide
if __name__ == '__main__':
import doctest
doctest.testmod()
Sidenote: Pylint yells at me for using i
instead of idx
.
Shout out to:
@SuperBiasedMan --For list comprehensions and truthiness of Python
@Caridorc --For using interactive python doc tests (made error checking a breeze)
@Mast --For explaining that more branches are horrible!
@Hurkyl --For the .extend()
method on lists! Very useful and easy to read
@DarioP --For clear instructional points, in English, for improving code and pitfalls to avoid
@mkrieger1 --For pointing out odd indentations that I never caught!
coding
line is indented by two spaces and the rest of the code by additional two spaces, both are superfluous. Also in line 40,pairs.append(num)
is indented one level (four spaces) too deep, which can be misleading, especially in Python. – mkrieger1 yesterdayCtrl-K
or pressing the{}
button, which indents everything uniformly by four spaces. – mkrieger1 yesterdayCtrl-K
and it did not work. So I eventually gave up. :( – Rafeh yesterday