Codilty, binary gap get the highest gap

Question

I have written a solution for the codility task which is Binary Gap. finding the highest gap in Binary. for example 1041 is the input number and convert it to binary the result is 5. here is the code :

package codility 
import(
 "strconv"
)
func Solutions(N int) int{
 binary := convertNumberToBinary(N)
 result := getHighestGap(binary)
 return result
}
func convertNumberToBinary(N int)string{
 result := ""
 for N > 0 {
 binary := N%2;
 result = strconv.Itoa(binary) + result
 N = N/2;
 }
 return result
}
func getHighestGap(binary string) int{
 result := 0
 count := 0 
 isCount := false
 stringArray := []byte(binary)
 for i:=0; i < len(stringArray); i++{
 if stringArray[i] == '1' {
 if result < count{
 result = count
 }
 count = 0
 isCount = true
 }else if isCount{
 //found 0
 count ++
 }
 }
 return result
}

and the test file :

package codility
import(
 "testing"
)
func TestBinrayGap(testing *testing.T){
 var testObjects = []struct{
 input int
 expected int
 }{
 {1041, 5},
 {529, 4},
 {20, 1},
 }
 for _, t := range testObjects {
 actual := Solutions(t.input)
 if actual != t.expected{
 testing.Errorf("Test failed expected : %s, actual : %s", t.expected, actual)
 }
 }
}

here I'm converting the input number to binary first and then count the highest length of the gap by converting the string to array byte `[]byte'. And for the test I'm using table driven test.

Answer 1

• It would have been a good idea to link to the problem statement in your question.
• Please run gofmt on your code, typically have your editor automatically format your files every time you save your source file.
• Your naming conventions aren't very Go-like.
  • Solutions is too vague, BinaryGap would be better.
  • convertNumberToBinary is really long; this isn't Java — toBinary would largely be enough
  • stringArray is very wrong: this variable is a byte slice, plus, it doesn't give you extra information from its type
  • TestBinryGap should probably be TestBinaryGap
  • testObjects is weird and slightly misleading; Go test files usually name this variable test or tc, and expected is pretty much always called want
• When you want to append repetitively to a string like in convertNumberToBinary, use string.Buffer. But here, why not just use a []bool that you can name isOne or something?
• I think the function is simple enough that you can just put it in one function, and compute the binary representation at the same time you compute the binary gap. This would make your memory complexity $(1) instead of O(log(n)) (no need to allocate a slice or a string anymore) and simplify your code.
• You don't need to convert your string (or your []bool) into a []byte later on; you can just iterate on it directly.
• You should probably test that the input is a nonnegative number and return an error if that's the case (although maybe this isn't necessary in a competitive programming setting).
• On the positive side, your test is a good idiomatic example of struct-based tests =)

• Gujarat Santana: I really like when you mention about stringArray, and the testObjects
• Gujarat Santana: but one thing that I still don't get it what do you mean by use []bool and name isOne ?
• Ted: You want to represent a binary number digit by digit. Rather than storing it as e.g. []byte("10011"), you can store it as []bool{true, false, false, true, true}. This way, you give the compiler the option of using only 1 bit by cell rather than 8 =) And you avoid making assumptions on your data: for example, if you store a binary number in a []byte, what if you have a bug and one of your cell ends up containing a different byte than '0' or '1'? This will be hard to track and debug.
• Gujarat Santana: ooh okay, I see, that's amazing thing suggestion. I can reduce the memory usage for the variable and would be easier to debug