6
\$\begingroup\$

Recently I was thinking about the keyless entry on my car and how simple it has to be, so I wrote a C# programme to validate a StreamingPin, that is, a pin that is entered character-by-character, and resets as soon as an invalid character is input. If the correct sequence of characters is entered, then the pin is validated as correct, but if another character is entered afterwards, the pin is incorrect.

Regardless of what happens before the correct sequence of characters, as soon as the valid sequence is entered the pin is valid.

class StreamingPin
{
    public bool IsCorrect => (_lastCorrect + 1) == _pin.Length;

    private int _lastCorrect = -1;
    private string _pin;

    public StreamingPin(string pin)
    {
        if (string.IsNullOrEmpty(pin))
        {
            throw new ArgumentException($"The value provided for {nameof(pin)} cannot be null or an empty string.");
        }

        _pin = pin;
    }

    public void ProcessCharacter(char c)
    {
        var currentIndex = _lastCorrect + 1;

        if (_pin[currentIndex] == c)
        {
            _lastCorrect = currentIndex;
        }
        else if (_pin[0] == c)
        {
            _lastCorrect = 0;
        }
        else
        {
            _lastCorrect = -1;
        }
    }
}

It takes \$O(1)\$ memory, and runs in \$O(n)\$ time where \$n\$ is the number of characters processed until the correct sequence is found.

No matter now many invalid characters are entered before a valid pin, there is no lockout procedure.

Test code:

var sp = new StreamingPin("PinNumber");
var input = "p2gsvpinnumberPinNumbessfeER#VvsdinNumberPinNumberdfvlj4kF4wfV";
var lastIndex = -1;

while (!sp.IsCorrect)
{
    lastIndex++;
    Console.WriteLine(input[lastIndex]);
    sp.ProcessCharacter(input[lastIndex]);
}

Console.WriteLine("Pin found");
\$\endgroup\$

1 Answer 1

5
\$\begingroup\$

_pin overflow

You're relying on the caller to prevent a buffer overflow. Consider if the calling code was this:

var sp = new StreamingPin("PinNumber");
var input = "p2gsvpinnumberPinNumbessfeER#VvsdinNumberPinNumberdfvlj4kF4wfV";
var lastIndex = -1;

while (!sp.IsCorrect)
{
    lastIndex++;
    Console.WriteLine(input[lastIndex]);
    sp.ProcessCharacter(input[lastIndex]);
}
sp.ProcessCharacter(input[lastIndex]);

Your ProcessCharacter method doesn't check that accessing _lastCorrect + 1 isn't going to overflow _pin. This seems like the responsibility of the StreamingPin class. I'd say, if they start adding letters after a valid pin, it should reset to not correct anymore (rather than throwing an exception that could give the caller information they might not know, such as how long the PIN is).

One way to achieve this would be to add a wrap to the increment:

var currentIndex = (_lastCorrect + 1) % _pin.Length;

readonly

Since you're not updating the pin after construction, I'd also consider making it readonly.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.