Streaming a Pin more natively

In my quest to better myself, it's time for the C++ version of my StreamingPin class, and all relevant helper classes.

The F# version: Streaming a Pin functionally(ish)

The C# version: Validating a StreamingPin

The F# version has no answers (yet) but I'm posting the C++ version as F# is a pretty inactive language here, so I'm not really concerned it's been left alone.

This one is actually pretty neat.

Basically, I did the same thing as the other two, but in C++ (Visual C++, for what that's worth). I built an abstract class for my ITests, and built my normal StreamingPin class, etc.

We'll start with the StreamingPin.h:

#pragma once
#include "ITests.h"
#include 

class StreamingPin
{
public:
    bool IsCorrect();
    bool ProcessCharacter(char c);
    StreamingPin(std::string pin);
    ~StreamingPin();
private:
    std::string _pin;
    int _lastCorrect;
};

class StreamingPinTests: public ITests
{
public:
    bool RunAll();
    std::string Name();
    int TestCount();
};

Just like all the others, the StreamingPinTests class is defined within the same file as StreamingPin to keep them together. (This will not be the case in the Java version.)

Then, StreamingPin.cpp:

```
#include "stdafx.h"
#include "StreamingPin.h"
#include
#include

bool StreamingPin::IsCorrect()
{
return (_lastCorrect + 1) == _pin.length();
}

bool StreamingPin::ProcessCharacter(char c)
{
if (IsCorrect())
{
_lastCorrect = -1;
}

int currentIndex = _lastCorrect + 1;

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

return IsCorrect();
}

StreamingPin::StreamingPin(std::string pin)
{
if (pin.length() == 0)
{
throw "The value provided for pin cannot be null or empty.";
}

_pin = pin;
_lastCorrect = 0;
}

StreamingPin::~StreamingPin()
{

}

bool StreamingPinTests::R

For toy program this is perfectly fine. It works, it's maintainable (could be better). But it's still far from being C++ code. I know that classes and inheritance are parts of C++, but they could easily be avoided. What the code lacks of is integrity with the language, e.g. it can be used with C++ standard library, at least it is painful to do so.

I didn't know what is the source of input, but since it is StreamingPin class, I'll use std::istream& as input argument type. Anyone who wants to input something else can easily create std::istream from whatever container they want. I will include it as a bonus at the end.

You did pretty good using std::string. I will not mention naming convention, since it's pretty opinion based.

class streaming_pin
{
    std::string pin;

So now we need to nail down what constructors we need. We want to make as few copies as possible. Your code currently makes 2 copies, one at the call side, and one to copy it into internal _pin string. To do this, we need a reference. We have 2 reference types in C++: lvalue reference and rvalue reference. They have very different meaning for humans, but they are the same for the compiler.

The convention is to write const lvalue reference (const T&) to define a copy constructor which performs deep copy. Rvalue references (T&&, be careful though, with templates T&& means something completely different) are used to write move constructors which perform shallow copy (e.g. steal resources). With this in mind, lets write value constructors.

public:
    streaming_pin(const std::string& pin_)
        :pin(pin_)
    {}

    streaming_pin(std::string&& pin_)
        :pin(std::move(pin_))
    {}

Why I didn't throw at zero length string? Because the class shouldn't care. It is the pin generator's problem if the pin is empty. It should throw at generator, not here. Even if the exceptions doesn't cost anything if they're not thrown (though there will be sensible performance hit when they're thrown, thus it's a bad idea to use them in performance critical code).

When writing any class, you should always consider if you need either of rule of zero, rule of 3 or rule of 5.

So, do we want our StreamingPin class to be copyable? Probably no. Do we want it to be movable? Probably yes (arguable). Do we want it to be copy assignable? No. Do we want it to be move assignable? Probably no as well.

streaming_pin(const streaming_pin& other) = delete;
streaming_pin& operator=(const streaming_pin& other) = delete;

streaming_pin(streaming_pin&& other) = default;
streaming_pin& operator=(streaming_pin&& other) = delete;

We don't need destructor since std::string takes care of itself and we don't need any other clean up logic.

The only thing left is what the class should really do: validate the pin from the stream.

Your logic in bool StreamingPin::ProcessCharacter(char c) is very strange. To the point that I'd say that you want a locker class that is unlockable and you can enter pin of it and query if it's already unlocked. I will assume that pin checker is intended.

bool validate(std::istream& instream)

Now we just traverse the contents of the stream (single pass), and check if contents are the same as pin:

auto currentpos = pin.begin();
    char buffer;
    while (currentpos != pin.end() && instream >> buffer)
    {
        if (buffer == *currentpos)
        {
            ++currentpos;
        }
    }

    return currentpos == pin.end();

Note that it is important to bare in mind that prefix operator++ matters for user defined types performace wise. I know it doesn't make any difference when working with built in types, but for user defined types operator++ is always made to be faster.

If you want to know if istream contains only the pin, you can write a check:

instream.get();
    bool exhausted = instream.eof();
    bool perfect_match = currentpos == pin.end();

    return exhausted && perfect_match;

Finally, full code:

```
#include
#include
#include

class streaming_pin
{
std::string pin;
public:
streaming_pin(const std::string& pin_)
:pin(pin_)
{}

streaming_pin(std::string&& pin_)
:pin(std::move(pin_))
{}

streaming_pin(const streaming_pin& other) = delete;
streaming_pin& operator=(const streaming_pin& other) = delete;

streaming_pin(streaming_pin&& other) = default;
streaming_pin& operator=(streaming_pin&& other) = delete;

bool validate(std::istream& instream)
{
auto currentpos = pin.begin();
char buffer;
while (currentpos != pin.end() && instream >> buffer)
{
if (buffer == *currentpos)
{
++currentpos;
}
}

instream.get();
bool exhausted = instream.eof();
instream.unget(); //put it back, because stream might be used later
bool perfect_match = currentpos == pin.end();

return exhaus

StackExchange Code Review Q#142093, answer score: 2