3
\$\begingroup\$

The codility.com free test is as follows:

A binary gap within a positive integer N is any maximal sequence of consecutive zeros that is surrounded by ones at both ends in the binary representation of N. For example, number 9 has binary representation 1001 and contains a binary gap of length 2. The number 529 has binary representation 1000010001 and contains two binary gaps: one of length 4 and one of length 3. The number 20 has binary representation 10100 and contains one binary gap of length 1. The number 15 has binary representation 1111 and has no binary gaps.

Write a function def solution(N) that, given a positive integer N, returns the length of its longest binary gap. The function should return 0 if N doesn't contain a binary gap.

For example, given N = 1041 the function should return 5, because N has binary representation 10000010001 and so its longest binary gap is of length 5.

My implementation is as follows:

def merge_step(x, y, result):

    if x[0] == 1:
        if y[0] == 0 and len(result) == 0:
            result.append(1)

    if x[0] == 0 and y[0] == 0:
        if len(result) > 0:
            result[len(result) -1] = result[len(result) -1] + 1


def divide_and_conquer(array, result):
    # [1, 0, 0, 0, 0, 0, 1, 0, 0, 0, 1]

    if len(array) == 1:
        return merge_step([array], [array], result)

    mid = len(array) // 2
    x = array[:mid]
    y = array[mid:]

    divide_and_conquer(x, result)
    divide_and_conquer(y, result)
    merge_step(x, y, result)

    if len(result) == 0:
        return 0
    return max(result)

def solution(N):
    # write your code in Python 2.7
    array = [int(x) for x in "{0:b}".format(N)]
    return divide_and_conquer(array, [])

print(solution(1041))
print(solution(15))

The online interpreter says it is correct, but I wanted to check whether there's a better way to do this.

Also, I wanted to verify that my running time is n log(n). The recursion is \$O(log n)\$ and the max is an \$O(n)\$ operation, correct?

\$\endgroup\$
  • \$\begingroup\$ You didn't like my suggestion in the previous review to rename x to left and y to right? \$\endgroup\$ – janos Mar 25 '17 at 19:37
  • \$\begingroup\$ @janos you make a good point. I didnt catch it on the last review \$\endgroup\$ – Sam Hammamy Mar 25 '17 at 19:47
1
\$\begingroup\$

Bug

What will be the value of merge_step([array], [array], result)? Nothing. The solution will be incorrect for 0 and 1.

Algorithm

Converting an integer to binary string, and then converting that to a list of integers (zeros and ones), and then slicing dicing lists is very inefficient.

A much more simple and efficient approach is to count the sequences of unset bits:

longest = 0
current_len = 0

while num > 0:
    if num & 1:
        if current_len > longest:
            longest = current_len
        current_len = 0
    else:
        current_len += 1
    num >>= 1

return longest

The reason why the time complexity of this is \$O(\log n)\$ is that we're counting bits, which grows much slower than the value of \$n\$.

Doctests

As I pointed out in the previous review, I strongly recommend to use doctests to verify your implementation.

\$\endgroup\$
  • \$\begingroup\$ Thanks again! How do I know if divide and conquer is the better approach vs something else? \$\endgroup\$ – Sam Hammamy Mar 25 '17 at 20:48
  • \$\begingroup\$ Also how do you recommend I improve my recursions? I am always finding that ending the recursion is not very clear \$\endgroup\$ – Sam Hammamy Mar 25 '17 at 20:49
  • 2
    \$\begingroup\$ Choose the approach by comparing their benefits and trade-offs. As for recursions, just keep practicing, and then it will start to click. Usually the terminating condition is the first thing to do at the top of the function, start with that and get it right. \$\endgroup\$ – janos Mar 25 '17 at 20:59

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.