0
\$\begingroup\$

I've implemented atoi function as described in leetcode problem string-to-integer-atoi.

I am looking for some feedback on writing this in a more scala native way and being able to include the requirement that:

If the numerical value is out of the range of representable values, INT_MAX (231 − 1) or INT_MIN (−231) is returned.

  object Solution {
    def myAtoi(str: String): Int = {

      val charMap = Map('1'->1, '2'->2, '3'->3, '4'->4, '5'->5, '6'->6, '7'->7, '8'->8, '9'->9, '0'-> 0)

      val keySet = charMap.keySet
      var num = 0
      var firstChar = -1
      var negFlag = false
      var zeroFlag = false

      var cleanStr = str.reverse.dropWhile(c => !charMap.contains(c) || c == '-' || c == ' ').trim

      for((c, i) <- cleanStr.view.zipWithIndex) {
        var summand = c match {
          case c if keySet.contains(c) => {
            if(firstChar == -1) firstChar = i
            charMap(c) * math.pow(10, i - firstChar).toInt
          }
          case '-' if(firstChar != -1) =>  {
            negFlag = true
            0
          }
          case _ => {
            zeroFlag = true
            0
          }
        }
        num += summand
      }
      if(zeroFlag) 0 else if(negFlag) num * -1 else num
    }
  }


\$\endgroup\$

1 Answer 1

1
\$\begingroup\$

A few observations about your code.

incomplete - As you've noted, it doesn't handle Int overflow. It also doesn't handle the optional leading + character.

digit characters - The variables charMap and keyMap are unnecessary. The .isDigit and .asDigit methods in the standard library provide the same functionality.

Boolean logic - Consider this Boolean test: !charMap.contains(c) || c == '-' || c == ' ' It translates to "if c is not a digit OR if c is a dash OR if c is a space." But if c is a dash or space then it is already not a digit. The dash and space tests serve no purpose because no dash or space character would get beyond the digit test.

unneeded vars - There's no reason to make cleanStr or summand a var. Their initial values are never changed. In fact, you could just drop summand altogether and increment num directly.

num += (c match { ...//rest of code block

excess braces - In a match{...} every case statement ends the previous code block and starts a new one. Therefore wrapping the code block in { braces } is redundant and just adds visual clutter. In other words, this...

case x =>
  //code
  //here

...is cleaner than this...

case x => {
  //code
  //here
}

style - The use of so much mutation (i.e. vars instead of vals) makes it pretty clear that you are using the Scala language to write C code. This will change as you become more familiar with the Scala standard library, the tenants of Functional Programming, and the study of other Scala code.

Here, for example, is a Scala solution for the atoi challenge.

val numRE = "\\s*([+-]?)0*(\\d+).*".r

def myAtoi(str :String) :Int = str match {
  case numRE(signStr, numStr) =>
    val sign    = if (Option(signStr).contains("-")) -1 else 1
    val bailOut = if (sign < 0) Int.MinValue else Int.MaxValue
    if (numStr.length > 10) bailOut                      //too many digits
    else if (numStr.length < 10 || numStr < "2147483648")
      sign * numStr.foldLeft(0)(_*10 + _.asDigit)        //calculate return Int
    else bailOut
  case _ => 0  //input doesn't match the pattern
}
\$\endgroup\$
1
  • \$\begingroup\$ Thanks for taking time to explain and also for your elegant solution :) \$\endgroup\$ Feb 24, 2020 at 4:33

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.