# Printing longest sequence of zeroes

I am doing a coding exercise in codility and I came across this question:

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:

class Solution { public int solution(int 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.

Assume that:

N is an integer within the range [1..2,147,483,647]. Complexity:

expected worst-case time complexity is O(log(N)); expected worst-case space complexity is O(1).

My code looks like this:

import java.util.*;

class Solution {
public static int solution(int N) {
return Optional.ofNullable(N)
.map(Integer::toBinaryString)
.filter(n -> n.length() > 1)
.map(t -> {
List<Integer> counts = new ArrayList<>();
int count = 0;
for(int i = 0; i < t.length(); i++)
{
if(t.charAt(i) == '0') {
count += 1;
} else if(count > 0) {
count = 0;
}
}
if(counts.size() > 0) {
Collections.sort(counts);
return counts.get(counts.size() - 1);
}
return 0;
})
.orElse(0);
}
}


What else can I do to improve the performance of the aforementioned code? How do I determine the big-O complexity of this program?

• Converting to a binary string and then operating on the string is much slower and more laborious than operating on the bytes directly.
• There is no need to collect all counts and then take the largest as we can just calculate the largest as we go.
• Give your class and method appropriate names.
• Check the precondition that the input is positive in the beginning of the method.

Here is a streamlined but untested solution:

class BinaryGapCalculator {
static int binaryGap(final int n) {
if (n <= 0)
throw new IllegalArgumentException("n must be positive; was " + n);
final int start = Integer.lowestOneBit(n), end = Integer.highestOneBit(n);
int curRun = 0, longestRun = 0;
final int bit = n & bitmask;
if (bit == 0)
++curRun;
else {
longestRun = Math.max(longestRun, curRun);
curRun = 0;
}
}
return Math.max(longestRun, curRun);
}
}


Regarding big-O, all solutions are technically O(1) because there are finitely many inputs. But if we were to pretend that an int could store arbitrarily many bits, the optimal solution would be θ(b) where b ~ log(n) is the number of bits of n. This is because the algorithm I wrote is θ(b) and the optimum must be at least as efficient as my algorithm; also there is no tighter bound than θ(b) because asymptotically, at least half of the input has to be seen. So my algorithm is optimal (up to a constant).

• ...and complexity is $O(n)$ Commented Mar 9, 2016 at 20:14
• For an arbitrary large int, if we pretend that instructions - including popcount - execute in constant time, there is a logarithmic solution.
– vnp
Commented Mar 9, 2016 at 21:51
• @vnp Constant popcount sounds unrealistic because in some sense it has to scan the whole number (but note that I counted my shift and mask as constant, which is technically incorrect, but I am assuming there would be a way to select a bit in constant time). Regardless, how would this logarithmic algorithm work? Commented Mar 9, 2016 at 22:01
• Sorry, got carried away. Of course there is no logarithmic solution. What I had in mind is that popcount((x ^ (x - 1)) - 1 gives the length of a run of zeroes from least significant in a constant time. Then shift right by the run length, negate, get rid of run of (used to be) ones and negate again to reach next run of zeroes. Surely still linear.
– vnp
Commented Mar 9, 2016 at 22:27
• Or change the way you use start and end in the loop. You can shift position instead of incrementing it, and use it directly as a bitmask. Commented Feb 5, 2019 at 9:02

## Optional abuse

In the code you shared, Optional is used only to create a chain of method calls and avoid writing conditional statements. But eventually you end up writing imperative logic with loops and conditionals anyway.

That not a proper way of using Optional.

Here's a quote from the answer by Stuart Marks, Java and OpenJDK developer:

Optional is intended to provide a limited mechanism for library method return types where there is a clear need to represent "no result," and where using null for that is overwhelmingly likely to cause errors.

...

The ability to chain methods from an Optional is undoubtedly very cool, and in some cases it reduces the clutter from conditional logic. But quite often this doesn't work out. A typical code smell is, instead of the code using method chaining to handle an Optional returned from some method, it creates an Optional from something that's nullable, in order to chain methods and avoid conditionals.

Additionally, plugging an int value into Optional.ofNullable() looks confusing, the boxed Integer wrapper produced out of it has no chance to be null.

## Avoid multiline lambdas

Multiline lambda expressions bear a lot of cognitive load and should be avoided.

Your map(t -> { ... }) basically hosts the bulk of your solution, this attempt to employ optional to create a method chain only creates unnecessary noise.

## Finding max element doesn't require sorting

Never use sorting to find the maximal (or minimal) element. It requires only a single iteration. And Collections class features max() method for that purpose.

And there's no need to store sizes of all gaps anyway, two variables are sufficient as shown in other reviews.

## Follow the Code conventions of the Java language

• Don't use C-style curly braces with an opening curly brace { on the next line (in Java, an opening curly brace should be placed on the same line with the declaration of a code element such as for-loop).

• There should be a white space between the keywords such as for and if and parenthesis (this rule accentuates the difference between the usage of keywords with parenthesis and method calls).

Here is the different way of implementation

Steps:

1. Convert the integer value to Binary format.
2. Iterate from first index to last index.
3. Check whether the value is 0. If yes, increment the current iteration value, if not calculate the max_zero value by comparing current iteration with existing max_zero value

Code:

public static int binaryGap(final int n) {
String binRep = Integer.toBinaryString(n);
int currentItr = 0;
int maxZeroSeq = 0;
for (int index = 0; index <binRep.length(); index ++) {
if (binRep.charAt(index) == '0') {
++currentItr;
} else {
maxZeroSeq = Math.max(maxZeroSeq, currentItr);
currentItr = 0;
}
}
if (currentItr!=0) {
if (currentItr > maxZeroSeq) {
return currentItr;
}
}
return Math.max(maxZeroSeq, currentItr);
}

• Try with N=28. It will return 2 which is wrong. Algorithm above returns 0 what is correct because gap should be surrounded with 1 from both sides Commented Feb 5, 2020 at 13:32
• You have presented an alternative solution, but haven't reviewed the code. Please edit to show what aspects of the question code prompted you to write this version, and in what ways it's an improvement over the original. It may be worth (re-)reading How to Answer. Commented May 21 at 8:57
• I agree @TobySpeight Commented May 21 at 10:00