Filtering a phone number

Question

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.

Answer 1

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"
}

• lmcphers: This is exactly what I was looking for, thank you. I do agree with your assessment of using if's and else instead of case statements - I was still learning Scala and practicing case statements as much as possible, but I'm starting now to grasp when and when not to use them. This has helped me in that area. The only question I have for you is for stationNumber. Why do you use drop(6).take(4) instead of takeRight(4) as it was originally? Was there a specific reason for this? You didn't mention it in your description, so I am assuming not, but just want to make sure.
• lmcphers: One other question now that I see you changed number to a val is regarding that. Is it appropriate to declare something as a value when it doesn't take arguments? Coming from an object-oriented background to a multi paradigm language definitely makes declaration choices such as val vs def and object, case class, and classes really interesting.