6
\$\begingroup\$

I've created a code generator generator, which is hosted here.

I need its parser portion reviewed for OOP, OOD, and C++ best practices.

gengenparser.h

#ifndef GENGENPARSER_H
#define GENGENPARSER_H
#include <string>
#include <boost/algorithm/string.hpp>
#include "linecodegenerator.h"
#include "staticcodegetter.h"
#include "codeappender.h"
#include "codejoiner.h"
#include "postparser.h"

enum SingleLineParseMode {
    LINEMODE_CODE,
    LINEMODE_TEMPLATE
};

enum BlockType {
    BLOCK_PREHEADER,
    BLOCK_HEADER,
    BLOCK_FOOTER,
    BLOCK_POSTFOOTER,
    BLOCK_CODE,
    BLOCK_UNKNOWN
};

enum BlockMode {
    BLOCKMODE_CODE,
    BLOCKMODE_TEMPLATE
};

static std::string TOKEN_LINEDUMP("$$$");
    static std::string TOKEN_INDENTNEXT("$$>");
static std::string TOKEN_INDENTEQUAL("$=>");
static std::string TOKEN_INDENTDEPTHOFTWO("$>>");

static std::string TOKEN_UNINDENTNEXT("<$$");
static std::string TOKEN_UNINDENTEQUAL("<=$");
static std::string TOKEN_UNINDENTDEPTHOFTWO("<<$");

static std::string TOKEN_INLINE_START("{$$");
    static std::string TOKEN_INLINE_END("$$}");

static std::string TOKEN_PREHEADER("$$PREHEADER");
    static std::string TOKEN_HEADER("$$HEADER");
static std::string TOKEN_FOOTER("$$FOOTER");
    static std::string TOKEN_POSTFOOTER("$$POSTFOOTER");
static std::string TOKEN_CODEBLOCK("$$CODE");
    static std::string TOKEN_ENDBLOCK("$$END");

class GenGenParser {
private:
    LineCodeGenerator* mLinecode;
    StaticCodeGetter* mStaticGetter;
    PostParser* mPostParser;
    CodeAppender mAppender;
    unsigned int indentCount;
    bool IsStrIn(std::string& str, int pos, std::string& checkStr);
    void LineModeParse(std::string& line, int size);

public:
    GenGenParser(LineCodeGenerator* linecode, StaticCodeGetter* staticGetter, PostParser* postParser);
    void Parse();
    void PostParse();
};

#endif // GENGENPARSER_H

gengenparser.cpp

#include "gengenparser.h"

bool GenGenParser::IsStrIn(std::string& str, int pos, std::string& checkStr)
{
    int check_str_size = checkStr.length();
    int str_size = str.length();
    if (pos + check_str_size > str_size) {
        return false;
    }
    for (int i = 0; i < check_str_size; i++) {
        if (str[pos + i] != checkStr[i]) {
            return false;
        }
    }
    return true;
}

void GenGenParser::LineModeParse(std::string& line, int size)
{
    std::string token("");
    SingleLineParseMode mode = LINEMODE_CODE;
    mLinecode->StartLine();
    for (int i = 0; i < size; ++i) {
        if (this->IsStrIn(line, i, TOKEN_INLINE_START)) {
            i += 2;
            if (!token.empty()) {
                mLinecode->WriteCodePrintingCode(token);
                token.clear();
            }
            mode = LINEMODE_TEMPLATE;
            continue;
        } else if (this->IsStrIn(line, i, TOKEN_INLINE_END)) {
            i += 2;
            if (!token.empty()) {
                mLinecode->WriteCode(token);
                token.clear();
            }
            mode = LINEMODE_CODE;
            continue;
        }

        char chr = line[i];
        if (mode == LINEMODE_CODE) {
            mLinecode->EscapedAppend(token, chr);
        } else {
            token.push_back(chr);
        }
    }

    mLinecode->WriteCodePrintingCode(token);
    mLinecode->EndLine();
    mAppender.AppendToCodeBody(mLinecode->CalculateIndent(indentCount) + mLinecode->GetGeneratedCode());
}

GenGenParser::GenGenParser(LineCodeGenerator *linecode, StaticCodeGetter *staticGetter, PostParser *postParser)
{
    mLinecode = linecode;
    mStaticGetter = staticGetter;
    mPostParser = postParser;
    indentCount = staticGetter->GetStartingIndent();

}

void GenGenParser::Parse()
{
    std::string line;
    BlockMode blockMode = BLOCKMODE_TEMPLATE;
    BlockType blockType = BLOCK_UNKNOWN;
    while (std::getline(std::cin, line)) {

        std::string trimmedLine = boost::trim_copy(line);
        if (boost::equal(trimmedLine, TOKEN_ENDBLOCK)) {
            blockMode = BLOCKMODE_TEMPLATE;
            blockType = BLOCK_UNKNOWN;
            continue;
        } else if (boost::equal(trimmedLine, TOKEN_PREHEADER)) {
            blockMode = BLOCKMODE_CODE;
            blockType = BLOCK_PREHEADER;
            continue;
        } else if (boost::equal(trimmedLine, TOKEN_HEADER)) {
            blockMode = BLOCKMODE_CODE;
            blockType = BLOCK_HEADER;
            continue;
        } else if (boost::equal(trimmedLine, TOKEN_FOOTER)) {
            blockMode = BLOCKMODE_CODE;
            blockType = BLOCK_FOOTER;
            continue;
        } else if (boost::equal(trimmedLine, TOKEN_POSTFOOTER)) {
            blockMode = BLOCKMODE_CODE;
            blockType = BLOCK_POSTFOOTER;
            continue;
        } else if (boost::equal(trimmedLine, TOKEN_CODEBLOCK)) {
            blockMode = BLOCKMODE_CODE;
            blockType = BLOCK_CODE;
            continue;
        }

        if (blockMode == BLOCKMODE_CODE) {
            switch (blockType) {
            case BLOCK_PREHEADER:
                mAppender.AppendToPreHeader(line);
                break;
            case BLOCK_HEADER:
                mAppender.AppendToHeader(line);
                break;
            case BLOCK_FOOTER:
                mAppender.AppendToFooter(line);
                break;
            case BLOCK_POSTFOOTER:
                mAppender.AppendToPostFooter(line);
                break;
            case BLOCK_CODE:
                mAppender.AppendToCodeBody(line);
                break;
            default:
                break;
            }
            continue;
        }

        if (this->IsStrIn(line, 0, TOKEN_LINEDUMP)) {
            mAppender.AppendToCodeBody(mLinecode->CalculateIndent(indentCount)
                                       + line.substr(TOKEN_LINEDUMP.length()));
            continue;
        } else if (this->IsStrIn(line, 0, TOKEN_INDENTNEXT)) {
            mAppender.AppendToCodeBody(mLinecode->CalculateIndent(indentCount++)
                                       + line.substr(TOKEN_INDENTNEXT.length()));
            continue;
        } else if (this->IsStrIn(line, 0, TOKEN_INDENTEQUAL)) {
            mAppender.AppendToCodeBody(mLinecode->CalculateIndent(++indentCount)
                                       + line.substr(TOKEN_INDENTEQUAL.length()));
            continue;
        } else if (this->IsStrIn(line, 0, TOKEN_INDENTDEPTHOFTWO)) {
            mAppender.AppendToCodeBody(mLinecode->CalculateIndent(++indentCount)
                                       + line.substr(TOKEN_INDENTDEPTHOFTWO.length()));
            indentCount++;
            continue;
        } else if (this->IsStrIn(line, 0, TOKEN_UNINDENTNEXT)) {
            mAppender.AppendToCodeBody(mLinecode->CalculateIndent(indentCount)
                                       + line.substr(TOKEN_UNINDENTNEXT.length()));
            if (indentCount > 0) {
                --indentCount;
            }
            continue;
        } else if (this->IsStrIn(line, 0, TOKEN_UNINDENTEQUAL)) {
            if (indentCount > 0) {
                --indentCount;
            }
            mAppender.AppendToCodeBody(mLinecode->CalculateIndent(indentCount)
                                       + line.substr(TOKEN_UNINDENTEQUAL.length()));

            continue;
        } else if (this->IsStrIn(line, 0, TOKEN_UNINDENTDEPTHOFTWO)) {
            if (indentCount > 0) {
                --indentCount;
            }
            mAppender.AppendToCodeBody(mLinecode->CalculateIndent(indentCount)
                                       + line.substr(TOKEN_UNINDENTDEPTHOFTWO.length()));
            if (indentCount > 0) {
                --indentCount;
            }
            continue;
        }
        this->LineModeParse(line, line.length());
    }
}
void GenGenParser::PostParse()
{
    CodeJoiner joiner(mAppender, mStaticGetter);
    mPostParser->PostParse(joiner.GetCode());
}

codeappender.h

#ifndef CODEAPPENDER_H
#define CODEAPPENDER_H
#include <string>
class CodeAppender {
private:
    std::string mStdStrPreHeader;
    std::string mStdStrHeader;
    std::string mStdStrCodeBody;
    std::string mStdStrFooter;
    std::string mStdStrPostFooter;

public:
    virtual void AppendToPreHeader(const std::string& code);
    virtual std::string GetPreHeader();
    virtual void AppendToHeader(const std::string& code);
    virtual std::string GetHeader();
    virtual void AppendToCodeBody(const std::string& code);
    virtual std::string GetCodeBody();
    virtual void AppendToFooter(const std::string& code);
    virtual std::string GetFooter();
    virtual void AppendToPostFooter(const std::string& code);
    virtual std::string GetPostFooter();
};

#endif // CODEAPPENDER_H

codejoiner.h

#ifndef CODEJOINER_H
#define CODEJOINER_H
#include <string>
#include "codeappender.h"
#include "staticcodegetter.h"

class CodeJoiner {
private:
    std::string mStdStrCode;

public:
    CodeJoiner(CodeAppender codeAppender, StaticCodeGetter* staticCodeGetter);
    virtual std::string GetCode();
};

#endif // CODEJOINER_H

postparser.h

#ifndef POSTPARSER_H
#define POSTPARSER_H
#include <iostream>
#include <string>
class PostParser {
public:
    virtual void PostParse(std::string code);
};

#endif // POSTPARSER_H

staticcodegetter.h

#ifndef STATICCODEGETTER_H
#define STATICCODEGETTER_H
#include <string>

class StaticCodeGetter {
public:
    virtual std::string GetBeforePreHeader() = 0; 
    virtual std::string GetAfterPreHeader() = 0; 
    virtual std::string GetAfterHeader() = 0; 
    virtual std::string GetBeforeFooter() = 0;
    virtual std::string GetAfterFooter() = 0;
    virtual std::string GetAfterPostFooter() = 0;
    virtual unsigned int GetStartingIndent() = 0;
};

#endif

linecodegenerator

#ifndef LINECODEGENERATOR_H
#define LINECODEGENERATOR_H
#include <string>

class LineCodeGenerator {
public:
    virtual void StartLine() = 0;
    virtual void EndLine() = 0;
    virtual void EscapedAppend(std::string& token, char c) = 0;
    virtual void WriteCodePrintingCode(const std::string& escapedCodeToPrint) = 0;
    virtual void WriteCode(const std::string& code) = 0;
    virtual std::string GetGeneratedCode() = 0;
    virtual std::string CalculateIndent(unsigned int amount) = 0;
};
#endif // LINECODEGENERATOR_H
\$\endgroup\$
  • 2
    \$\begingroup\$ Note: Indentation issue in the first file is not intentional, somehow it is rendered that way, and I cannot fix it. \$\endgroup\$ – 422_unprocessable_entity Feb 4 '15 at 14:40
4
\$\begingroup\$

const is good to use on token that do not change, just in case.

e.g.

static const std::string TOKEN_LINEDUMP("$$$");

You class GenGenParser contains a number of raw pointers e,g, LineCodeGenerator* which could be replaced with smart pointers to make ownership clear and memory handling easier.

std::unique_ptr<LineCodeGenerator> mLineCode;
...

Prefer references instead of pointers when you pass arguments to functions, that way you are sure in the function that they are defined and not null.

Also you see that it is not clear from the function prototype

GenGenParse(LineCodeGenerator* linecode, ... )

whether ownership is to be passed to it or not, using a smart pointer eliminates that need.

e.g. here you directly that GenGenParse only shares the object, it does not delete it.

GenGenParse(shared_ptr<LineCodeGenerator> linecode, ...);

There seems to be no error handling in your code, maybe it would be useful to printout a syntax error and in which location it occurred.

Prefer to put implementation details at the end of the class declaration like private/protected parts. In a perfect world when a user wants to use your class he should not need to know the implementation.

In general I find your code nicely structured.

EDIT: rephrased according to comment, hopefully making it more clear:

When you declare a class put the private and protected parts below the public part because the user of the class should need to know about implementation details (design goal).

class X
{
public:
...
protected:
...
private:
...
};

or go one step further and use the pimpl idiom and move all implementation details away from the class declaration to the .cpp file.

example:

class CodeAppender {
public:
...
private:
    struct sections;
    std::unique_ptr<sections> sectionsImpl;
...

in the cpp file

struct sections
{
  std::string mStdStrPreHeader;
  std::string mStdStrHeader;
  std::string mStdStrCodeBody;
  std::string mStdStrFooter;
  std::string mStdStrPostFooter;
};

CodeAppender::CodeAppender() : sectionsImpl(std::make_unique<sections>())
{}
\$\endgroup\$
3
\$\begingroup\$
static std::string TOKEN_LINEDUMP("$$$");
        static std::string TOKEN_INDENTNEXT("$$>");
static std::string TOKEN_INDENTEQUAL("$=>");
static std::string TOKEN_INDENTDEPTHOFTWO("$>>");

static std::string TOKEN_UNINDENTNEXT("<$$");
static std::string TOKEN_UNINDENTEQUAL("<=$");
static std::string TOKEN_UNINDENTDEPTHOFTWO("<<$");

static std::string TOKEN_INLINE_START("{$$");
        static std::string TOKEN_INLINE_END("$$}");

static std::string TOKEN_PREHEADER("$$PREHEADER");
        static std::string TOKEN_HEADER("$$HEADER");
static std::string TOKEN_FOOTER("$$FOOTER");
        static std::string TOKEN_POSTFOOTER("$$POSTFOOTER");
static std::string TOKEN_CODEBLOCK("$$CODE");
        static std::string TOKEN_ENDBLOCK("$$END");

I'm fairly sure C++ also have IDE's with auto-formatting support. Use them to streamline your indentation.

        switch (blockType) {
        case BLOCK_PREHEADER:
            mAppender.AppendToPreHeader(line);
            break;
        case BLOCK_HEADER:
            mAppender.AppendToHeader(line);
            break;
        case BLOCK_FOOTER:
            mAppender.AppendToFooter(line);
            break;
        case BLOCK_POSTFOOTER:
            mAppender.AppendToPostFooter(line);
            break;
        case BLOCK_CODE:
            mAppender.AppendToCodeBody(line);
            break;
        default:
            break;
        }

Seems like you could do with a method or function mAppender.Append(line, blockType)?

\$\endgroup\$
  • 2
    \$\begingroup\$ That's actually a copy paste issues. It is not indented at all. \$\endgroup\$ – 422_unprocessable_entity Feb 4 '15 at 14:35
2
\$\begingroup\$
  1. You have many Get... functions, none of which are const. All getter functions should be made const member functions.

    The idea behind const objects and const member function is too long to be included here as an answer. More can be found at Wikipedia and other resources on the web. Briefly, when an object is declared const, it is meant to be read-only. When a member function is declared const, in that function, the object on which the function is called is meant to be read-only.

  2. CodeAppender has many virtual functions even though it doesn't derive from any base class and it is not a base class of any other classes. I would remove the virtual specifier from those functions unless there is a need for it.

\$\endgroup\$
  • \$\begingroup\$ @JaDogg, see my updated answer. \$\endgroup\$ – R Sahu Feb 6 '15 at 16:37

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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