HiveBrain v1.2.0
Get Started
← Back to all entries
patternMinor

Filtering a phone number

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
numberphonefiltering

Problem

I am learning Scala still and refactoring some old code. I was hoping the community here could give me pointers to fix this code up since there are some issues I have with it visually, but don't know if there's an elegant solution for my vision.

class PhoneNumber(phonenumber:String) {
  def number: String = {
    phonenumber.filter(_.isDigit) match {
      case x if x.startsWith("1") && x.length == 11 => x.substring(1)
      case x if x.length != 10 => "0000000000"
      case x => x
    }
  }

  def areaCode: String = number.take(3)

  override def toString: String =
    "(" + areaCode + ") " + number.substring(3,6) + "-" + number.takeRight(4)
}


I want to refactor my case statement as much as possible because right now I think it looks kind of strange. One thing I am particularly not fond of is the case x => x, but I am wondering what others think about my approach in general and what I might do to make this code better visually and functionally.

Solution

One of the main changes you'll probably want to make is to change number and areaCode from functions to values. As your code is right now, every time you use number it is having to recompute your top code block. The same goes for areaCode.

At first glance I don't see a reasonable way to eliminate the conditional statements that you step though to calculate number. Because of that, there isn't a great way to utilize pattern matching. So IMO you may as well take advantage of the fact that conditional statements are expressions in Scala.

Another small tweak I made in calculating number was to change "0000000000" to "0" * 10. Its not so hard to count out ten zeros, but if you needed a larger amount of them you now know about this other method.

I added class fields for the other subsections of a typical phone number, e.g. exchangeCode and stationNumber.

Finally I utilized string interpolation for your toString method as to my mind it makes the expression easier to mentally parse.

class PhoneNumber(phoneNumber: String) {
  val number = {
    val tmpNumb = phoneNumber.filter(_.isDigit) 
    if (tmpNumb.startsWith("1") && tmpNumb.length == 11) tmpNumb.drop(1)
    else if (tmpNumb.length != 10) "0" * 10
    else tmpNumb
  }

  val areaCode      = number.take(3)
  val exchangeCode  = number.drop(3).take(3)
  val stationNumber = number.drop(6).take(4)

  override def toString = s"($areaCode) $exchangeCode-$stationNumber"
}

Code Snippets

class PhoneNumber(phoneNumber: String) {
  val number = {
    val tmpNumb = phoneNumber.filter(_.isDigit) 
    if (tmpNumb.startsWith("1") && tmpNumb.length == 11) tmpNumb.drop(1)
    else if (tmpNumb.length != 10) "0" * 10
    else tmpNumb
  }

  val areaCode      = number.take(3)
  val exchangeCode  = number.drop(3).take(3)
  val stationNumber = number.drop(6).take(4)

  override def toString = s"($areaCode) $exchangeCode-$stationNumber"
}

Context

StackExchange Code Review Q#94143, answer score: 4

Revisions (0)

No revisions yet.