Question

Just for kicks I decided to write a snippet that takes an integer and converts it into binary. It seems to be performing the conversion but I was wondering if there's anything that I might be missing from this. It will be great to get some feedback.

Thanks

def convertToBinary(n: Int, bin: List[Int]): String = {
  if(n/2 == 1) {
    (1::(n%2)::bin).mkString(" ")
  } else {
     val r = n%2;val q = n/2;converttobinary(q,r::bin)
  }
}
Was it helpful?

Solution 2

1) Formatting :-)

def converttobinary(n:Int, bin:List[Int]):String = {
  if(n/2 == 1) (1:: (n % 2) :: bin).mkString(" ")
  else {
    val r = n % 2;
    val q = n / 2;
    converttobinary(q, r::bin)
  }
}

2) Signature:

I would omit convert part, made accumulator optional parameter (function user doesn't have to supply arguments used only for internal implementation, right?)

def toBinary(n:Int, bin: List[Int] = List.empty[Int]): String = {
  if(n/2 == 1) (1:: (n % 2) :: bin).mkString(" ")
  else {
    val r = n % 2
    val q = n / 2
    toBinary(q, r::bin)
  }
}

Now it can be used as:

val str = toBinary(42)

Somebody might propose you to pimp such function, so call might look like

val str = 42.toBinary // btw, there is such function in std lib: 42.toBinaryString

But I don't see much profit.

The other thing that bothers me is that you're using List & mkString for this purpose, why not StringBuilder? What are the q and r?

And final minor point -- place @annotation.tailrec to ensure that function will be optimized even in case of any future changes

OTHER TIPS

def toBinary(n: Int): String = n.toBinaryString

1) A short one:

scala> def toBinary(n: Int): String = n match {
     |   case 0|1 => s"$n"
     |   case _   => s"${toBinary(n/2)}${n%2}"
     | }
toBinary: (n: Int)String

scala> toBinary(155)
res0: String = 10011011

Tail recursion

def toBinary(n: Int): String = {
  @tailrec def binary(acc: String, n: Int): String = {
    n match {
      case 0 | 1 => n+acc
      case _ => binary((n % 2)+acc, (n / 2))
    }
  }
  binary("",n)
}
Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top