ToString() and AddString() method without using .Net Collections

I was asked to answer the following question before setting up a phone interview, but they said my code wasn't detailed enough.

Question:


Without using any of the .NET Collection or Linq libraries (i.e.
without using List), implement a ListOfStrings object that
contains a set of strings. This object has the following methods:

Add(string) - inserts a string to the end of the list ToString() -
Returns the list as a comma separated string.


Example Usage:

ListOfStrings list = new ListOfStrings(); // set is empty
list.Add("abc"); // set is "abc"
list.Add("xyz"); // set is now "abc", "xyz"
list.Add("123"); // set is now abc", "xyz", "123"
list.ToString(); // Should return "abc,xyz,123"

Ideally, this code should be able to handle large numbers of items.

Answer:

namespace ConsoleApplication3
{
    class Program
    {
        static void Main(string[] args)
        {
            while (true)
            {
                ListOfStrings ls = new ListOfStrings();
                ls.Add("test1");
                ls.Add("test2");
                ls.Add("122");
                ls.Add("");
                ls.ToString();
                Console.WriteLine(ls.ToString());
            }
        }
    }

    public class ListOfStrings
    {
        private string ListOfString;

        public ListOfStrings()
        {
            ListOfString = null;
        }
        public string Add(string input)
        {
            if (input != string.Empty)
            {
                ListOfString = ListOfString == null ? input : ListOfString + "," + input;
            }
            return ListOfString;
        }        

        public string ToString()
        {
            return ListOfString;
        }
    }
}

Please let me know feedback on the code.

-
ListOfString = null; is useless, fields get initialized to default(T) which is null for reference types. So you can drop the constructor.

For simple initializations you can use inline initializers. e.g.

private string ListOfString = "";

-
You're hiding ToString() instead of overriding it.

• You're ignoring empty input strings, the spec doesn't require that. Curiously you're not ignoring null input strings.

-
Using a string as accumulator leads to quadratic runtime since it has to create a new string which contains all current data for each append.

Use StringBuilder.Append in Add instead. Don't return a string from Add, else you lose the performance advantage. Call stringBuilder.ToString() in your ToString operation. This has linear runtime.

• You're returning null if no value has ever been added. I'd expect the empty string instead.

I don't know what they meant by calling your code "not detailed enough". You could add documentation for corner cases, possibly in the form of unit tests. For my observations 3 and 5 it's not clear if your code is behaving like you intended. Documentation and/or unit tests would have clarified your intent.

StackExchange Code Review Q#46017, answer score: 20