Formatting a string by converting to a number then back to a string

Question

I'm not sure whether this qualifies as code smell, or if there may be some better way of performing the same task, but, basically, I want to format a 10 digit string, using C#, and have the following options...

Option 1

public string FormatNumber(string value)
{
    int i;
    if(int.TryParse(value, out i)) return i.ToString("000 000 0000");
    else throw new InvalidArgumentException("'" + value + "' is not a valid argument.");
}

Option 2

public string FormatNumber(string value)
{
    int i;
    if(int.TryParse(value, out i)) return value.Insert(6," ").Insert(3," ");
    else throw new InvalidArgumentException("'" + value + "' is not a valid argument.");
}

Both achieve the correct result, but they both feel a little ... smelly.

Am I being too pedantic, or is their a clear winner?

Answer 1

Personally I think the first one looks more readable. It make really clear what's the expected output. The second one may take some more effort.

By the way, what happens if the number does not fit the 10 digits ? And what if the number is negative ?

Answer 2

One trick you may not have thought of is to give the special number its own class, with implicit conversion, which makes it easier to use.

The special class provides a natural extension point if in the future you need to add comparison operations, custom hash code, custom serialization, etc.

Also note-- an int can't hold all ten digit numbers, you'd need a long. Also, you should validate against negative numbers.

public class MyNumber
{
    protected readonly long _value;

    public MyNumber(string source)
    {
        if (!long.TryParse(source, out _value)) throw new ArgumentException("String cannot be converted to a MyNumber");
        if (_value <=0) throw new ArgumentException("MyNumber must be at least 1.");
        if (_value > 9999999999) throw new ArgumentException("MyNumber must be ten digits or fewer.");
    }

    static public implicit operator long(MyNumber source)
    {
        return source.Value;
    }

    static public implicit operator MyNumber(string source)
    {
        return new MyNumber(source);
    }

    public long Value { get { return _value; } }

    public override string ToString()
    {
        return _value.ToString("000 000 0000");
    }
}

Some ways you could use it:

MyNumber n = "12345678";
Console.WriteLine("Your magic number is {0}", n);

Console.WriteLine("Another number is {0}", (MyNumber)"9876543");

Output:

Your magic number is 001 234 5678
Another number is 000 987 6543

Link to DotNetFiddle