# Find largest binary gap of a number N improved

This is a follow-up question based on the original one. Could you please see the updated and refactored version and point at things to be improved in term of logic or efficiency?

I have now know as I have been made aware about the Greedy Algorithm; Math.Max and binary shifting which deal more gracefully with the task, however, if I were in a position to be unaware of them and this is the code I managed to write, what's your thoughts on it?

    public static int FindGap(int number)
{
string toBinary = Convert.ToString(number, 2);
char[] toChar = toBinary.ToCharArray();

char previous = '0';
char current = '0';
int maxGap = 0;

for (int i = 0; i < toChar.Length; i++)
{
if (toChar[i] == '1')
{
current = toChar[i];

// keep last/latest largest gap as an answer
{
}

// reset if 1 appeared and start counting next gap
maxGap = 0;
}
if ((toChar[i] == '0' && current == '1') || (toChar[i] == '0' && previous == '0'))
{
current = '0';
maxGap++;
previous = current;
}
}

{
}
else return 0;
}

• The code still creates a string although this is not needed. You should try the code from my answer on the previous question. It is much more efficient. May 24, 2019 at 13:52
• @RolandIllig: The OP states that yours and other suggestions are ignored on purpose. I'm not claiming that the justification is particularly reasonable. May 24, 2019 at 14:14
• Long story short, I wanted to see my solution improved with the approach I have undertaken initially, without knowing what was already invented to support me in resolving such problems. It was important for me to see that I can do the Convert.ToString() inside the foreach loop and also that I have failed the obvious binary logic. To me that helps a lot.
– Kris
May 24, 2019 at 14:50

If I were in a position to be unaware of them and this is the code I managed to write, what's your thoughts on it?

• No need to call ToCharArray() because you can iterate over the chars of a string as well
• By using an else if instead of the second if you won't need to check for toChar[i] == '0' but basically you only need a else.
• Using a foreach loop instead of a for loop seems more natural for this task.
• The returning of the answer can be simply return answer;
• Because ToString(int, 2) strips leading '0' you can remove both current and previous.

Summing up it could look like so

public static int FindGap(int number)
{
int maxGap = 0;

foreach(var c in Convert.ToString(number, 2))
{
if (c == '1')
{
{
}
maxGap = 0;
}
else
{
maxGap++;
}
}