Creating Phone Number challenge from Code Wars

Question

I solved a question on Codewars about creating a phone number:

Write a function that accepts an array of 10 integers (between 0 and 9), that returns a string of those numbers in the form of a phone number. Example

createPhoneNumber([1, 2, 3, 4, 5, 6, 7, 8, 9, 0]) // => returns "(123) 456-7890"

The returned format must be correct in order to complete this challenge.

Don't forget the space after the closing parentheses!

Here's my solution how to improve i struggled with this solution to came up a lot but at the end i completed it i have some problems when i try to iterate through each integer because code makes duplicates but i fixed it with slicing and code works , for example where i've had a problem:

My Solution--->(111) 1111-1111

Solution how should look like--->(111) 111-1111

But i figured it out for better solutions and tell me if its code it's good or i should change something!

def create_phone_number(n):
    res = "("
    x_string = "".join([str(x) for x in n])
    x_new_string = "".join(x_string)
    for i in range(len(n)):
        if i == 2:
            res += x_new_string[i] + ") "
        if i == 5:
            res += x_new_string[i] + "-"
        else:
            res += x_new_string[i]
    return res[0:5] + " " + res[7:]

Answer 1

Loop like a native

You are using range(len(n)) to iterate over n using an index. This is similar to how iterations are handled in most languages but in Python, there is almost always a better way to write this. I highly recommend the talk "Look like a native" which explains this in details.

In you particular case, using enumerate makes things much cleaner, faster and more Pythonic:

def create_phone_number2(n):
    res = "("
    for i, c in enumerate([str(x) for x in n]):
        if i == 2:
            res += c + ") "
        if i == 5:
            res += c + "-"
        else:
            res += c
    return res[0:5] + " " + res[7:]

I took this chance to remove the call to join as per ades' answer.

String handling

It seems weird to have some logic to format res and yet have some more code to reformat res at the end by selecting the relevant parts of the string.

At the end of the day, it looks like this last step is just a workaround for a bug in a different part of the code: a character is introduced twice in the res and must we removed.

The issue is that when i == 2, we add the character twice:

• first with res += x_new_string[i] + ") "
• then with res += x_new_string[i]

Using elif makes things cleaner and solves the issue:

def create_phone_number2(n):
    res = "("
    for i, c in enumerate([str(x) for x in n]):
        if i == 2:
            res += c + ") "
        elif i == 5:
            res += c + "-"
        else:
            res += c
    return res

More simplification

Adding c could be done in a single place. Also, this could be the place where we convert to string.

def create_phone_number2(n):
    res = "("
    for i, c in enumerate(n):
        res += str(c)
        if i == 2:
            res += ") "
        elif i == 5:
            res += "-"
    return res

Other options

Instead of messing with indices, an alternative could be to rely on the features from Python such as string formatting. For instance, one could write:

def create_phone_number3(n):
    return "(%d%d%d) %d%d%d-%d%d%d%d" % (n[0], n[1], n[2], n[3], n[4], n[5], n[6], n[7], n[8], n[9])

This can probably be improved further for instance, using Greedo's suggestion:

def create_phone_number3(n):
    return "({}{}{}) - {}{}{} {}{}{}{}".format(*n)

Answer 2

It would be useful if you wrote out what the actual problem you're trying to solve is, because now I have to guess based on your code.

The part about x_new_string = "".join(x_string) looks a bit odd since you've joined the array of characters into a string already for x_string. The variable names are not very good - I have no idea what n or res is, and neither x_string nor x_new_string tells me very much.

You can just iterate over the elements of the n sequence (not sure what type of sequence it is based on your code alone, but it doesn't matter much) with for element in n instead of reading its length and then using range.

You can/should also do elif (instead of if) for the == 5 check; surely i is never both 2 and 5, and currently you will enter else also if i == 5 (since that isn't part of the same if-else group).

Answer 3

Aside from the possible optimizations already addressed in other answers, there are 2 important things missing from your code: documentation and input validation.

Documentation

Documentation serves 2 purposes: improving the readability of your code and communicating with future users of your code.

For the first one, it is usually good enough to rely on self-documenting code, starting with good variable names. In your case, your variable names convey little to no information, or are downright misleading: I would expect n to be an integer, following usual mathematical conventions, when you actually expect it to be a list of integers.

The function name is also a poor fit, and although I understand you didn't choose it, I believe format_phone_number would be much better: this function doesn't create a phone number, in fact it takes an existing phone number in some format as an input.

You can also include type hints to communicate what comes in and out of your function.

As such, a function signature would become something like:

def format_phone_number(digits: list) -> str:

Which, in itself, conveys a lot about the intent of the function.

There are still some information missing for anyone to be able to use your function: the fact that the input should be a list of exactly 10 single digits, and how the output is formatted. Note that the latter is not obvious, for example an American user would expect a 10-digit phone number to be formatted (012) 345-6789, whereas a French user would expect 01 23 45 67 89.

As such, you should include a docstring with your function, fully specifying your function's behavior.

Input validation

While Code Wars is probably well behaved and won't call your function with invalid parameters, you can't expect that from all users. Proper documentation will help, but nothing would prevent calling your function with unexpected parameters (something else than a list, a list with more of fewer characters than expected, a list containing out-of-range integers...). Even type hints are just hints, Python will call the function with anything the caller provides.

As such, it is good practice to verify that the arguments passed match what your logic can handle: in this case, you should verify that the argument is a list with 10 items, that these items are ints, and that they are in range [0-9].

Example code

Here is my take on the exercise. I decided to use a formatted string litteral -- aka f-string -- for the actual formatting logic, as I think this is the most straightforward method, but other approaches can be equally valid, as long as the code does what it should.

def create_phone_number(digits: list) -> str:
    '''
    Format a phone number represented as a list of digit into a string
    formatted according to the American phone numbers convention

    Parameters
    ----------
    digits : list
        A list of exactly 10 digits representing a phone number

    Returns
    -------
    str
        The formatted phone number

    >>> create_phone_number([1, 2, 3, 4, 5, 6, 7, 8, 9, 0])
    '(123) 456-7890'
    '''
    if len(digits) != 10:
        raise ValueError("Invalid number of digits (10 required)")
    if not all(isinstance(d, int) and 0 <= d <= 9 for d in digits):
        raise ValueError("'digits' must be a list of integers in range [0, 9]")

    digits_as_string = ''.join([str(d) for d in digits])
    return f'({digits_as_string[0:3]}) {digits_as_string[3:6]}-{digits_as_string[6:]}'

Note that I stuck to the create_phone_number name, as following the given specifications takes priority over my own naming preferences.