# Clean up improperly formatted phone numbers

This is a problem from Exercism here, the goal is to clean up badly formatted phone numbers. The rules are:

• If the phone number is less than 10 digits assume that it is a bad number
• If the phone number is 10 digits assume that it is good
• If the phone number is 11 digits and the first number is 1, trim the 1 and use the first 10 digits
• If the phone number is 11 digits and the first number is not 1, then it is a bad number
• If the phone number is more than 11 digits assume that it is a bad number

A function should pretty-print the numbers in the form (123) 456-7890.

Here is my solution. I'm very new to Clojure and am sure it's not idiomatic in parts. For example, should I have stored the different parts of the number as separate variables?

I would also be interested in tips on how to format the code to make it easier to read, I'm only aware of lispy-multiline which I find a bit too aggressive.

All feedback much appreciated.

(ns phone-number)

(defn digit-not-one-or-zero [digit]
(and (not (= \0 digit))
(not (= \1 digit))))

(defn clean-number [num-string]
;; strip all non-digit characters from a string
(filter #(Character/isDigit %) num-string))

(defn check-valid [num-string]
;; number is valid only if:
;; a) length is between 10-11 chars
;; b) if 11 chars, first digit must be 1
;; c) (after stripping first digit if 11), first and fourth can't be 0 or 1
(let [clean-string (clean-number num-string)
len (count clean-string)]
(if (or (< len 10) (> len 11))
false
(if (= len 11)
(if (= \1 (first clean-string))
(check-valid (rest clean-string))
false)
(if (and (digit-not-one-or-zero (first clean-string))
(digit-not-one-or-zero (nth clean-string 3)))
true
false)))))

(defn number [num-string]
(if (check-valid num-string)
(let [clean-string (clean-number num-string)]
(if (= (count clean-string) 10)
(apply str (clean-number clean-string)) (apply str (rest (clean-number clean-string)))))
"0000000000"))

(defn area-code [num-string]
;; get the first 3 chars of the cleaned up number
(apply str (take 3 (number num-string))))

(defn pretty-print [num-string]
(let [formatted-number (number num-string)]
(apply str (concat
"(" (area-code num-string) ")" " " (subs formatted-number 3 6) "-" (subs formatted-number 6)))))


I suggest you to look at Clojure Cheatsheet and get familiar with the functions dedicated for working with strings. There is also the library clojure.string, containing a few more. With these functions, you won't have to convert your string into sequence and then back repeatedly.

Take for example your pretty-print- subs returns string, concat returns sequence, apply str returns string again. Pick the correct data structure at the beginning, use dedicated functions and convert as little as possible.

Use :require in ns to include clojure.string library:

(ns phone-number
(:require [clojure.string :as s])
(:gen-class))


I will start with the area-code. This function could be improved with ->> (thread-last macro) and clojure.string/join like this:

(defn area-code [num-string]
(->> (number num-string)
(take 3)
s/join))


But you can also use subs:

(defn area-code [num-string]
(subs (number num-string) 0 3))


Next, pretty-print. You can use format here:

(defn pretty-print [num-string]
(let [validated (number num-string)]
(format "(%s) %s-%s" (area-code validated)
(subs validated 3 6)
(subs validated 6))))


Next, digit-not-one-or-zero. You usually create a set with these elements and call it on your argument, something like:

(defn digit-not-one-or-zero [digit]
(not (#{\0 \1} digit)))


Or even shorter:

(def digit-not-one-or-zero (complement #{\0 \1}))


Your clean-number is ok (I would just add s/join at the end to ensure it will return a string, as any other function here), unless you want to use some regex:

(defn clean-number [num-string]
(s/replace num-string #"\D" ""))


Next, number and check-valid.

• Don't use nested ifs- learn about cond, condp and case and use these. When can be also useful here.
• Don't use (if ... true false). In Clojure, all values are logically true or false. The only "false" values are false and nil - all other values are logically true. So, take this example:
(if (and (digit-not-one-or-zero (first clean-string))
(digit-not-one-or-zero (nth clean-string 3)))
true
false)


You can just write:

(and (digit-not-one-or-zero (first clean-string))
(digit-not-one-or-zero (nth clean-string 3)))

• check-valid is too long- you should break it into smaller functions.

So, here is my version you can study. Note how using short functions, idioms and proper flow-control macros increased readability:

(ns phone-number
(:require [clojure.string :as s])
(:gen-class))

(def allowed-digits (complement #{\0 \1}))

(defn clean-number [num-string]
(s/replace num-string #"\D" ""))

(defn validate10 [s]
(and (allowed-digits (.charAt s 0))
(allowed-digits (.charAt s 3))
s))

(defn validate11 [s]
(when (= \1 (.charAt s 0))
(validate10 (subs s 1))))

(defn validate [num-string]
(condp = (count num-string)
11 (validate11 num-string)
10 (validate10 num-string)
false))

(defn number [num-string]
(if-let [valid (validate (clean-number num-string))]
valid
"0000000000"))

(defn area-code [num-string]
(subs (number num-string) 0 3))

(defn pretty-print [num-string]
(let [validated (number num-string)]
(format "(%s) %s-%s" (area-code validated)
(subs validated 3 6)
(subs validated 6))))


EDIT: Alternative solution with regexes:

(defn number [num-string]
(if-let [valid (->> (s/replace num-string #"\D" "")
(re-matches #"1?([^01]..[^01].{6})")
second)]
valid
"0000000000"))

(defn area-code [num-string]
(subs (number num-string) 0 3))

(defn pretty-print [num-string]
(s/replace (number num-string)
#"(\d{3})(\d{3})(\d{4})"
"($1)$2-\$3"))

• Thanks so much, this is all really helpful. Do you have any suggestions for rules for formatting? Dec 18, 2022 at 19:38