String Token Generator for ExpressGenGen

Question

StringTokenGenerator is an interface (or abstract class) I've created to the specific task of generating a string token of a given character set. It allows appending characters to it and then it can be used to receive a string token. I wrote this for the second version of ExpressGenGen a code generator generator. Second version uses TDD as the development practice.

At first there is the header only abstract class, and after that I have included it's implementations, and then Test class. I'm using Google Mock and Google Test for testing.

Note that I'm generating the TestHelper. See here to see how it's done.

What I want reviewed:

1. Am I following C++/C++11 idioms correctly?
2. Does it need a better API design?
3. Is there better ways to utilize Google Mock and Google Test?
4. Is it too much abstraction?
5. Does this adhere to the const correctness principle?
6. Am I following good object oriented design principles?
7. Do I need to apply DRY principle somewhere in this code?
8. Am I following good practices when It comes to TDD?
9. Can the test cases be written better?
10. Am I missing any corner cases in the test cases?

StringTokenGenerator.h

#ifndef _STRING_TOKEN_GENERATOR_H_
#define _STRING_TOKEN_GENERATOR_H_
//--------------------------------------------
// Has
//
//--------------------------------------------
// Inherits
//
//--------------------------------------------
// Uses
#include <string>
//--------------------------------------------

class StringTokenGenerator {
public:
    virtual void Append(char c) = 0;
    virtual std::string GetToken() = 0 ;
    virtual ~StringTokenGenerator() {}

private:
};

//-------------------------------------------

#endif // _STRING_TOKEN_GENERATOR_H_

DoubleQuoteStringTokenGenerator.h

#ifndef _DOUBLE_QUOTE_STRING_TOKEN_GENERATOR_H_
#define _DOUBLE_QUOTE_STRING_TOKEN_GENERATOR_H_

//--------------------------------------------
// Has
//
//--------------------------------------------
// Inherits
#include "StringTokenGenerator.h"
//--------------------------------------------
// Uses
#include <memory>
//--------------------------------------------

class DoubleQuoteStringTokenGenerator : public StringTokenGenerator {
public:
    DoubleQuoteStringTokenGenerator();
    virtual void Append(char c);
    virtual std::string GetToken();

private:
    std::unique_ptr<std::string> mToken;
};

//-------------------------------------------

#endif // _DOUBLE_QUOTE_STRING_TOKEN_GENERATOR_H_

DoubleQuoteStringTokenGenerator.cpp

//--------------------------------------------
// Definition
#include "DoubleQuoteStringTokenGenerator.h"
//--------------------------------------------
// Uses
//
//--------------------------------------------

DoubleQuoteStringTokenGenerator::DoubleQuoteStringTokenGenerator()
    : mToken(new std::string())
{
}

void DoubleQuoteStringTokenGenerator::Append(char c)
{
    switch (c) {
    case '"':
        mToken->append("\\\"");
        break;
    case '\\':
        mToken->append("\\\\");
        break;
    default:
        mToken->push_back(c);
        break;
    }
}

std::string DoubleQuoteStringTokenGenerator::GetToken()
{
    std::string token("\"");
    token.append(*mToken);
    token.append("\"");
    mToken->clear();
    return token;
}

//--------------------------------------------

SingleQuoteStringTokenGenerator.h

#ifndef _SINGLE_QUOTE_STRING_TOKEN_GENERATOR_H_
#define _SINGLE_QUOTE_STRING_TOKEN_GENERATOR_H_

//--------------------------------------------
// Has
//
//--------------------------------------------
// Inherits
#include "StringTokenGenerator.h"
//--------------------------------------------
// Uses
#include <string>
#include <memory>
//--------------------------------------------

class SingleQuoteStringTokenGenerator : public StringTokenGenerator {
public:
    SingleQuoteStringTokenGenerator();
    virtual void Append(char c);
    virtual std::string GetToken();

private:
    std::unique_ptr<std::string> mToken;
};

//-------------------------------------------

#endif // _SINGLE_QUOTE_STRING_TOKEN_GENERATOR_H_

SingleQuoteStringTokenGenerator.cpp

//--------------------------------------------
// Definition
#include "SingleQuoteStringTokenGenerator.h"
//--------------------------------------------
// Uses
//
//--------------------------------------------

SingleQuoteStringTokenGenerator::SingleQuoteStringTokenGenerator()
    : mToken(new std::string())
{
}

void SingleQuoteStringTokenGenerator::Append(char c)
{
    switch (c) {
    case '\'':
        mToken->append("\\'");
        break;
    case '\\':
        mToken->append("\\\\");
        break;
    default:
        mToken->push_back(c);
        break;
    }
}

std::string SingleQuoteStringTokenGenerator::GetToken()
{
    std::string token("'");
    token.append(*mToken);
    token.append("'");
    mToken->clear();
    return token;
}

//--------------------------------------------

TestStringTokenGenerator.cpp

//--------------------------------------------
// Definition
//
//--------------------------------------------
// Uses
#include "TestHelper.h"
//--------------------------------------------

// ---->>> Double Quote <<<----

TEST(StringTokenGenerator, DoubleQuoteStringTokenGeneratorBasicTest)
{
    auto tokenGen(std::make_shared<DoubleQuoteStringTokenGenerator>());
    ASSERT_EQ(tokenGen->GetToken(), "\"\"");
    ASSERT_EQ(tokenGen->GetToken(), "\"\"");
    ASSERT_EQ(tokenGen->GetToken(), "\"\"");
    ASSERT_EQ(tokenGen->GetToken(), "\"\"");
    tokenGen->Append('h');
    tokenGen->Append('e');
    tokenGen->Append('l');
    tokenGen->Append('l');
    tokenGen->Append('o');
    // Should translate to {"hello"}
    ASSERT_EQ(tokenGen->GetToken(), "\"hello\"");
    ASSERT_EQ(tokenGen->GetToken(), "\"\"");
    ASSERT_EQ(tokenGen->GetToken(), "\"\"");
    ASSERT_EQ(tokenGen->GetToken(), "\"\"");
    ASSERT_EQ(tokenGen->GetToken(), "\"\"");
    ASSERT_EQ(tokenGen->GetToken(), "\"\"");
}

TEST(StringTokenGenerator, DoubleQuoteStringTokenGeneratorEscapeDoubleQuote)
{
    auto tokenGen(std::make_shared<DoubleQuoteStringTokenGenerator>());
    tokenGen->Append('"');
    tokenGen->Append('o');
    tokenGen->Append('n');
    tokenGen->Append('e');
    tokenGen->Append('"');
    // Should translate to {"\"one\""}
    ASSERT_EQ(tokenGen->GetToken(), "\"\\\"one\\\"\"");
    ASSERT_EQ(tokenGen->GetToken(), "\"\"");
}

TEST(StringTokenGenerator, DoubleQuoteStringTokenGeneratorEscapeAll)
{
    auto tokenGen(std::make_shared<DoubleQuoteStringTokenGenerator>());
    tokenGen->Append('"');
    tokenGen->Append('C');
    tokenGen->Append(':');
    tokenGen->Append('\\');
    tokenGen->Append('"');
    // Should translate to {"\"C:\\\""}
    ASSERT_EQ(tokenGen->GetToken(), "\"\\\"C:\\\\\\\"\"");
    ASSERT_EQ(tokenGen->GetToken(), "\"\"");
}

TEST(StringTokenGenerator, DoubleQuoteEmptyFromBegining)
{
    auto tokenGen(std::make_shared<DoubleQuoteStringTokenGenerator>());
    ASSERT_EQ(tokenGen->GetToken(), "\"\"");
    ASSERT_EQ(tokenGen->GetToken(), "\"\"");
    ASSERT_EQ(tokenGen->GetToken(), "\"\"");
    ASSERT_EQ(tokenGen->GetToken(), "\"\"");
    ASSERT_EQ(tokenGen->GetToken(), "\"\"");
    ASSERT_EQ(tokenGen->GetToken(), "\"\"");
    ASSERT_EQ(tokenGen->GetToken(), "\"\"");
    ASSERT_EQ(tokenGen->GetToken(), "\"\"");
    ASSERT_EQ(tokenGen->GetToken(), "\"\"");
}

// ---->>> Single Quote <<<----

TEST(StringTokenGenerator, SingleQuoteStringTokenGeneratorBasicTest)
{
    auto tokenGen(std::make_shared<SingleQuoteStringTokenGenerator>());
    ASSERT_EQ(tokenGen->GetToken(), "''");
    ASSERT_EQ(tokenGen->GetToken(), "''");
    ASSERT_EQ(tokenGen->GetToken(), "''");
    ASSERT_EQ(tokenGen->GetToken(), "''");
    tokenGen->Append('h');
    tokenGen->Append('e');
    tokenGen->Append('l');
    tokenGen->Append('l');
    tokenGen->Append('o');
    // Should translate to {'hello'}
    ASSERT_EQ(tokenGen->GetToken(), "'hello'");
    ASSERT_EQ(tokenGen->GetToken(), "''");
    ASSERT_EQ(tokenGen->GetToken(), "''");
    ASSERT_EQ(tokenGen->GetToken(), "''");
    ASSERT_EQ(tokenGen->GetToken(), "''");
}

TEST(StringTokenGenerator, SingleQuoteStringTokenGeneratorEscapeSingleQuote)
{
    auto tokenGen(std::make_shared<SingleQuoteStringTokenGenerator>());
    tokenGen->Append('\'');
    tokenGen->Append('o');
    tokenGen->Append('n');
    tokenGen->Append('e');
    tokenGen->Append('\'');
    // Should translate to {'\'one\''}
    ASSERT_EQ(tokenGen->GetToken(), "'\\'one\\''");
    ASSERT_EQ(tokenGen->GetToken(), "''");
}

TEST(StringTokenGenerator, SingleQuoteStringTokenGeneratorEscapeAll)
{
    auto tokenGen(std::make_shared<SingleQuoteStringTokenGenerator>());
    tokenGen->Append('\'');
    tokenGen->Append('C');
    tokenGen->Append(':');
    tokenGen->Append('\\');
    tokenGen->Append('\'');
    // Should translate to {'\'C:\\\''}
    ASSERT_EQ(tokenGen->GetToken(), "'\\'C:\\\\\\''");
    ASSERT_EQ(tokenGen->GetToken(), "''");
}

TEST(StringTokenGenerator, SingleQuoteEmptyFromBegining)
{
    auto tokenGen(std::make_shared<SingleQuoteStringTokenGenerator>());
    ASSERT_EQ(tokenGen->GetToken(), "''");
    ASSERT_EQ(tokenGen->GetToken(), "''");
    ASSERT_EQ(tokenGen->GetToken(), "''");
    ASSERT_EQ(tokenGen->GetToken(), "''");
    ASSERT_EQ(tokenGen->GetToken(), "''");
    ASSERT_EQ(tokenGen->GetToken(), "''");
    ASSERT_EQ(tokenGen->GetToken(), "''");
    ASSERT_EQ(tokenGen->GetToken(), "''");
}
//--------------------------------------------

Answer 1

No names starting with underscore followed by uppercase:

A name like this:

_STRING_TOKEN_GENERATOR_H_

Is using a reserved naming convention, and thus should not be used. You could safely be using just STRING_TOKEN_GENERATOR_H.

Verbose comments can easily become visual pollution:

Your sentry comments / markers are way too verbose. Some are not even serving a purpose, as they mark empty sections:

//--------------------------------------------
// Has
//
//--------------------------------------------
// Inherits
//
//--------------------------------------------
// Uses
#include <string>
//--------------------------------------------

That's a huge waste of lines and also quite distracting. Please trim them down.

Use default for empty virtual destructor:

C++11 introduced default member functions. Instead of defining an empty virtual destructor, such as in:

virtual ~StringTokenGenerator() {}

You can default it, which is a more up-to-date style:

virtual ~StringTokenGenerator() = default;

Use override for overridden virtual methods:

C++11 also introduced the very useful override keyword, which can be applied to virtual methods overridden by a child class. Doing so will improve compiler diagnostics if you shadow a member function name by accident, and could potentially allow for some extra compile-time optimizations related to de-virtualization.

You can add override to any virtual method implemented by child classes of the StringTokenGenerator interface.

Don't use pointers/dynamic-memory if you don't have to:

In your child classes (or implementations if you will), you are using a pointer to a string (mToken). There doesn't seem to be any need for that, you could have just declared the string by value. That would also result in a slightly more performance efficient program.

The use of pointers is usually associated with extending the lifetime of an object beyond its scope of declaration. Prefer declaring by value where viable.

Answer 2

Repeated tests

TEST(StringTokenGenerator, SingleQuoteEmptyFromBegining)
{
    auto tokenGen(std::make_shared<SingleQuoteStringTokenGenerator>());
    ASSERT_EQ(tokenGen->GetToken(), "''");
    ASSERT_EQ(tokenGen->GetToken(), "''");
    ASSERT_EQ(tokenGen->GetToken(), "''");
    ASSERT_EQ(tokenGen->GetToken(), "''");
    ASSERT_EQ(tokenGen->GetToken(), "''");
    ASSERT_EQ(tokenGen->GetToken(), "''");
    ASSERT_EQ(tokenGen->GetToken(), "''");
    ASSERT_EQ(tokenGen->GetToken(), "''");
}

What is this testing?

Must a token generator somehow be able to create exactly 8 tokens?

I feel you should write one test that tests the base - that a new single quote string token generator gives you the proper empty string. And one test that tests that, given a value, the first token will be the newly proper token, and the next will be an empty token. Another test tests whether you handle the special tokens properly. That way, you have tests that test functionality, rather than behavior...