patterncppMinor
Code Generator Generator
Viewed 0 times
codegeneratorstackoverflow
Problem
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
gengenparser.cpp
```
#include "gengenparser.h"
bool GenGenParser::IsStrIn(std::string& str, int pos, std::string& checkStr)
{
int check_str_size
I need its parser portion reviewed for OOP, OOD, and C++ best practices.
gengenparser.h
#ifndef GENGENPARSER_H
#define GENGENPARSER_H
#include
#include
#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("%%CODEBLOCK_0%%gt;>");
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_Hgengenparser.cpp
```
#include "gengenparser.h"
bool GenGenParser::IsStrIn(std::string& str, int pos, std::string& checkStr)
{
int check_str_size
Solution
const is good to use on token that do not change, just in case.
e.g.
You class
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
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.
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).
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:
in the cpp file
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 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 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 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())
{}Code Snippets
static const std::string TOKEN_LINEDUMP("$$$");std::unique_ptr<LineCodeGenerator> mLineCode;
...GenGenParse(LineCodeGenerator* linecode, ... )GenGenParse(shared_ptr<LineCodeGenerator> linecode, ...);class X
{
public:
...
protected:
...
private:
...
};Context
StackExchange Code Review Q#79577, answer score: 4
Revisions (0)
No revisions yet.