Let me fix thou number

Problem Statement:


Write a program that cleans up user-entered phone numbers so that they
can be sent SMS messages.


The rules are as follows:

• If the phone number is less than 10 digits assume that it is 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 last 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

We've provided tests, now make them pass.


Hint: Only make one test pass at a time. Disable the others, then flip
each on in turn after you get the current failing one to pass.

Code:

public class PhoneNumber {
  private String number;

  public PhoneNumber(String number) {
    this.number = cleanNumber(number);
    setNumber();
  }

  public String getNumber() {
    return number;
  }

  public String getAreaCode() {
    return number.substring(0, 3);
  }

  public String pretty() {
    return String.format(
        "(%s) %s-%s",
        getAreaCode(), number.substring(3, 6), number.substring(6));
  }

  private void setNumber() {
    if (hasInvalidInput(number)) {
      number = "0000000000";
    } else if (number.length() == 11) {
      number = number.substring(1);
    }
  }

  private boolean hasInvalidInput(String number) {
    return hasInvalidLength(number) || hasInvalidPrefix(number);
  }

  private boolean hasInvalidLength(String number) {
    return (number.length()  11);
  }

  private boolean hasInvalidPrefix(String number) {
    return number.length() == 11 && !number.startsWith("1");
  }

  private static final String cleanNumber(String number) {
    return number.replaceAll("[\\s\\.()-]", "");
  }
}

Test Suite:

```
import org.junit.Test;

import static org.junit.Assert.*;

public class PhoneNumberTest {

@Test
public void cleansNumber() {

Make it immutable

In cases when you're forced to work in the constructor,
you might want to do all the work there.
On the other hand,
make the result of the calculations final,
and the object created immutable.

Your implementation was effectively immutable.
The value of number never change after construction time.
But this was not really guaranteed.
One could still accidentally add a statement to change number.
You can improve this, by making the number field final.

And if number is final,
then there's no point calculating area code either,
or the pretty format,
all these could be pre-calculated at construction time.

Pointless static final

The static and final modifiers don't make much sense together.
You can drop the final safely, as a static method cannot be overridden anyway.

More static methods

Many of the helper methods don't need access to the fields,
so could have been static.

The invalid number

The invalid number is not exactly trivial.
Instead of having it buried deep in the implementation,
it would be better to move it to a constant.

Suggested implementation

Putting the above suggestions together:

public class PhoneNumber {
    public static final String INVALID_NUMBER = "0000000000";

    private final String number;
    private final String areaCode;
    private final String prettyFormat;

    public PhoneNumber(String number) {
        this.number = cleanNumber(number);
        this.areaCode = this.number.substring(0, 3);
        this.prettyFormat = prettyFormat(this.number, areaCode);
    }

    public String getNumber() {
        return number;
    }

    public String getAreaCode() {
        return areaCode;
    }

    public String pretty() {
        return prettyFormat;
    }

    private static String prettyFormat(String number, String areaCode) {
        return String.format("(%s) %s-%s", areaCode, number.substring(3, 6), number.substring(6));
    }

    private static boolean hasInvalidLength(String number) {
        return (number.length()  11);
    }

    private static String cleanNumber(String number) {
        number = number.replaceAll("[\\s\\.()-]", "");

        if (hasInvalidLength(number)) {
            return INVALID_NUMBER;
        }
        if (number.length() == 11) {
            if (number.charAt(0) != '1') {
                return INVALID_NUMBER;
            }
            return number.substring(1);
        }
        return number;
    }
}

StackExchange Code Review Q#127724, answer score: 3