CareerCup (Bloomberg): Check if string is valid based on brackets

Question

The goal of this problem is to determine if a string with brackets is valid. Below is a summary of the question from careercup.

Check if string s is valid based on brackets

"(({{}}))" is a valid s 
"{[]}" is a valid s
"[{[}]]" is not valid

I know this is a rather basic exercise. I am looking for feedback on whether my solution is sound and efficient. I would also like to know if my code is clean. Also is there too much for a 45 minute interview? The solution is shown below.

public class Question_ValidStringWithBrackets
{
    public bool IsValidBracketString(String s, BracketValidator validator = null)
    {
        if (s == null)
            throw new ArgumentNullException("A string is required");

        if (validator == null)
            validator = BracketValidator.CreateDefaultValidator();

        Stack<char> openStack = new Stack<char>();

        foreach (var curBracket in s)
        {
            if (validator.IsOpen(curBracket))
            {
                openStack.Push(curBracket);
            }
            else if (openStack.Count == 0 || validator.IsMatchingPair(openStack.Pop(), curBracket))
            {
                return false;
            }
        }
        return openStack.Count == 0;
    }

    public class BracketValidator
    {
        private HashSet<char> openBrackets = new HashSet<char>();
        private Dictionary<char, char> closedOpenedPair = new Dictionary<char, char>();

        public void AddPair(char open, char close)
        {
            if (char.IsWhiteSpace(open) || char.IsWhiteSpace(close))
                throw new ArgumentException("A bracket must be specified. An empty character is not allowed.");

            if (openBrackets.Contains(open) || openBrackets.Contains(close))
                throw new ArgumentException("Brackets exist already.");

            if (closedOpenedPair.ContainsKey(open) || closedOpenedPair.ContainsKey(close))
                throw new ArgumentException("Brackets exist already.");

            openBrackets.Add(open);
            closedOpenedPair.Add(close, open);
        }

        public Boolean IsOpen(char open)
        {
            return openBrackets.Contains(open);
        }

        public bool IsMatchingPair(char open, char close)
        {
            return closedOpenedPair.TryGetValue(open, out char actualClose) && actualClose == close;
        }

        public static BracketValidator CreateDefaultValidator()
        {
            var validator = new BracketValidator();
            validator.AddPair('{', '}');
            validator.AddPair('[', ']');
            validator.AddPair('(', ')');

            return validator;
        }
    }
}

Answer 1

Guard conditions

ArgumentNullException is typically thrown on incorrect usage of the API. It is not an end-user exception, so I would not bother with an exception message.

throw new ArgumentNullException("A string is required");

throw new ArgumentNullException(nameof(s)); // perhaps rename 's' to 'input' or 'value'

For the ArgumentException instances you throw, I would also add the nameof(parameterName) to the constructor. A decent error message is relevant here, so keep the message.

You have a guard on whitespace. I don't see this in the spec. Is this a requirement? If not, allow white space as either an open or close bracket.

if (char.IsWhiteSpace(open) || char.IsWhiteSpace(close))

The next guard checks on duplicate registrations. An alternative policy is to ignore a registration if already registered. This depends on whether you want your API to be error-prone or act as a sandbox for consumers.

if (openBrackets.Contains(open) || openBrackets.Contains(close))

I'm missing a guard condition. I doubt open and close can be the same.

if (open == close)  // throw ..

Readability

You could substitute some if statements with a ternary operator.

s = s ?? throw new ArgumentNullException(nameof(s));
validator = validator ?? BracketValidator.CreateDefaultValidator();

Redundant type declarations can be replaced by var. Unlike javascript, var is ok in C#.

Stack<char> openStack = new Stack<char>();

var openStack = new Stack<char>();