6
\$\begingroup\$

I've been trying to improve my C++ skills, and deiced to try my hand at making a interpreter for a toy language. The language is called Quartz, and so far the only thing you can do is output strings. The following keywords can be used to print out a string: output, which prints all the strings on one line, and nl_output, which prints each string on a different line.

The following program is valid in Quartz:

nl_output "Hello World"
nl_output "Goodbye Wolrd"
nl_output "This is a test of the Quartz language"

Each file has the .qz extension, and is basically like a text file.

The over-view of how my interpreter works is:

  1. It first opens a .qz, and then checks if the file was opened successfully.
  2. After ensuring that the files has been opened properly, the file contents is then read into a string. The string is then feed to a lexer that checks for the tokens. The lexer use a for-loop to iterate over the string, and adds any tokens it finds to a vector.
  3. The lexer then returns the vector to be read by the parser. The parser uses a while loop to iterate over the vector, and calls the correct code if a keyword/keywords is found.

main.cpp

#include<iostream>
using std::cout;
using std::cerr;
using std::endl;
#include<fstream>
using std::ifstream;
using std::fstream;
#include<string>
using std::string;
using std::getline;
#include<vector>
using std::vector;

void open_file(const char *filename, ifstream &data)
{
    data.open(filename);
    if(data.fail())
    {
        cerr << "FileError: specified file '" << filename << "' could not be found" << endl;
    }
}

vector<string> lexer(string &data_str, ifstream &data)
{
    string tok;
    string string_var;
    string expr;

    vector<string> tokens;
    getline(data, data_str, '\0');
    bool is_string = false;
    data_str += '$';
    for(unsigned int i=0; i < data_str.length(); i++)
    {
        tok += data_str[i];
        if(tok[tok.size()-1] == '\n' or tok[tok.size()-1] == '$')
        {
            tok = "";
        }
        if(data_str[i] == ' ')
        {
            if(is_string == false)
            {
                tok = "";
            }
            else if(is_string == true)
            {
                tok = " ";
            }
        }
        if(tok == "nl_output")
        {
            tokens.push_back("nl_output");
            tok = "";
        }
        if(tok == "output")
        {
            tokens.push_back("output");
            tok = "";
        }
        if(data_str[i] == '"')
        {
            if(is_string == false)
            {
                is_string = true;
            }
            else if (is_string == true)
            {
                tokens.push_back("string:" + string_var);
                string_var = "";
                is_string = false;
                tok = "";
            }
        }
        if(is_string)
        {
            string_var += tok;
            tok = "";
        }
    }
    /*for(int i=0;i<tokens.size();i++)
    {
        cout << tokens[i] << ' ';
    }*/
    //cout << tokens[0] + " " + tokens[1].substr(0,6) << endl;
    return tokens;
}


void parser(const vector<string> &tokens)
{
    unsigned int i = 0;
    while(i < tokens.size())
    {
        if(tokens[i] + " " + tokens[i+1].substr(0,6) == "output string") 
        {
            if(tokens[i+1].substr(0,6) == "string")
                cout << tokens[i+1].substr(8, tokens[i+ 1].size());
            i+=2;
        }
        else if(tokens[i] + " " + tokens[i+1].substr(0,6) == "nl_output string")
        {
            if(tokens[i+1].substr(0,6) == "string")
                cout << tokens[i+1].substr(8, tokens[i+ 1].size()) << endl;
            i+=2;  
        }
    }
}

int main(int argc, char *argv[])
{
    ifstream data;
    open_file(argv[1], data);
    string data_str;
    vector<string> tokens = lexer(data_str, data);
    parser(tokens);
    return 0;
}

To test the interpreter, simply compile the code in your command prompt/terminal window.

In my case I did:

g++ C:\main.cpp -o quartz.exe

Then run the [insert executable name].exe. The .exe takes one command line argument, which is the path to your .qz file. To make the .qz file, make a text file, and choose to rename the extension .qz. Or if you don't want to go through that hassle, a .txt file works fine to.

The three main questions I have are:

  1. Is the way I'm reading over my string, and adding my tokens to the tokens vector inefficient and slow?
  2. Is it bad practice to read A file until a NULL character ('\0') is reached?
  3. Is it mandatory to close a file after opening it? What might occur if I choose not to?
\$\endgroup\$
  • \$\begingroup\$ Use an existing lexer and parser tools to generate the code you need. \$\endgroup\$ – Martin York Aug 11 '16 at 11:14
  • \$\begingroup\$ But i'm trying to make my own. That sort of the point :) \$\endgroup\$ – Christian Dean Aug 11 '16 at 13:10
  • \$\begingroup\$ You are trying to build an interpretor. That's completely different. Don't write the bits that are already easily built using other tools (The lexer/parser). It just distracts you from the actual interesting bit (the language). \$\endgroup\$ – Martin York Aug 11 '16 at 16:40
  • \$\begingroup\$ @LokiAstari If I'd ever get serious about writing my own language, I'd probably use a existing lexer and parser. I made my own for practice. \$\endgroup\$ – Christian Dean Aug 11 '16 at 17:29
4
\$\begingroup\$

The three main questions I have are:

OK.

Is the way I'm reading over my string, and adding my tokens to the tokens vector inefficient and slow?

It's not the worst I have seen. But there does seem to be an awful lot of copying going on. You don't have to actually send back strings as the tokens. The lexer usually reads the string and converts this into a stream (or a vector) of lexemes. The lexemes only need to be a stream of numbers.

nl_output => 256
output    => 257
<string>  => 258

But the worst part is that it is not clear what you are trying to achieve (without really digging into the code). Your code should be self documenting and currently is not.

Is it bad practice to read A file until a NULL character ('\0') is reached?

Yes. Because there can potentially be '\0' characters as valid input. Are you assuming that the a file is null terminated? It is not. When you reach the end of file the end of file flag will be set on the stream.

Is it mandatory to close a file after opening it? What might occur if I choose not to?

Not mandatory. In my opinion not good practice (unless you plan to do something if it fails). And closing a read file is not going to fail in an exciting way. Other things will have gone wrong first. Let the destructor of the stream close the file for you.

Code Review.

I think your lexer can be much more easily written.

  1. Assuming all lexemes are "white space separated".
  2. The list of lexemes is:
    • TERMINAL: nl_output
    • TERMINAL: output
    • Quoted String: -> "<Any character that is not ">*"

Code

 std::vector<std::string> lexer(std::istream& s)
 {
     std::vector<std::string> result;
     std::string word;
     while(s >> word)   // reads a word from the stream.
     {                  // Drops all proceeding white space.

         if (word == "nl_output") {
             result.push_back(word);
         }
         else if (word == "output") {
             result.push_back(word);
         }
         else if (word[0] == '"') {
             result.push_back(readComment(word, s));
         }
         else {
             // Error
         }
     }
     return result;
}
std::string readComment(std::string const& word, std::istream& s)
{
    // First see if the whole quote is in the first word.
    auto find = std::find(std::begin(word) + 1, std::end(word), '"');
    if (find != std::end(word))
    {
         auto extraStart = find+1;
         auto extraDist  = std::distance(extraStart, std::end(word));
         for(int loop = 0; loop < extraDist; ++loop)
         {
             s.unget();
         }
         return word.substr(0, std::distance(std::begin(word), extraStart));
    }

    // OK the quote spans multiple words.
    std::string moreData;
    std::getline(s, moreData, '"');
    return word + moreData + '"';
 }

But this logic will get convoluted real quickly. I suggest you use a real lexer (like flex). Writing the rules is much simpler.

Space              [ \r\n\t]
QuotedString       "[^"]*"
%%
nl_output          {return 256;}
output             {return 257;}
{QuotedString}     {return 258;}
{Space}            {/* Ignore */}
.                  {error("Unmatched character");}
%%
\$\endgroup\$
  • \$\begingroup\$ But isn't it a bad idea to do while(data >> word)? You trying to read data into word, and your trying to actively use word. \$\endgroup\$ – Christian Dean Aug 11 '16 at 17:47
  • \$\begingroup\$ What is wrong with that? I read a word then used it. \$\endgroup\$ – Martin York Aug 11 '16 at 18:29
  • \$\begingroup\$ I just thought that while your still trying to read the word, your trying to also use the word. But it works perfectly, so i guess i'm wrong. \$\endgroup\$ – Christian Dean Aug 11 '16 at 18:35
  • \$\begingroup\$ @Mr.Python The while(s >> word) happens first. The result of this operation is basically true/false depending if the read worked. If that succeeds (returns true) then the loop is entered. You can then modify or use the word as required. The while is not checked again until the loop is restarted. \$\endgroup\$ – Martin York Aug 11 '16 at 18:51
  • \$\begingroup\$ your code breaks when there is white-space in the file. It only reads the last word. Is there any solution for this? \$\endgroup\$ – Christian Dean Aug 11 '16 at 23:59
2
\$\begingroup\$
  1. I think your way is mostly hard to understand. See below.
  2. Probably not.
  3. I think in your case the destructor of the ifstream should take care of that when your main returns and leaves it's scope.

Additional Remarks: For readability, I would suggest you re-order your if-else constructs to be:

if(is_string)
{
  // do things
}
else
{
  // do other things
}

There does not seem to be any good reason to compare a bool once with true and then again with false. There is no other state. Next: I'm not quite sure where you're going with this, but do you have any reason to have the tokens be pushed like this?

if (tok == "nl_output")
{
    tokens.push_back("nl_output");
    tok = "";
}
if (tok == "output")
{
    tokens.push_back("output");
    tok = "";
}

If you really need to do that, this position in code is a good example for using an if - else if, since the token will never be both nl_output and output, especially if you reset it in the body of the if. Could you not identify tokens by the white spaces separating them?

I also think that your parser will be horribly hard to maintain in the long run. I had to use the debugger and look twice to understand the

if (tokens[i] + " " + tokens[i + 1].substr(0, 6) == "output string")

The second if-clauses in the bodies of your parser seem unnecessary since the first if already matched the concatenated strings.

Furthermore, does your main really need to have the file's content in its scope? Just wondering since I' would assume that the lexer just tokenizes the input and raises an error if it encounters invalid tokens. For that, of course, you could define a set of keywords - output and nl_output seem like good candidates.

I hope I could help.

\$\endgroup\$
  • \$\begingroup\$ Thanks for the insight. I'll look at the problems your noted. I by no means, am good at c++, and wan tot know the beast practices. \$\endgroup\$ – Christian Dean Aug 11 '16 at 13:08
  • \$\begingroup\$ The reason I used the second if statement, check for the word string, is that i plan to expand this parser and need to be able to tell the difference between strings, integers, and expressions. \$\endgroup\$ – Christian Dean Aug 11 '16 at 13:20
  • \$\begingroup\$ The second nested if would still be true whenever the first surrounding if is true. \$\endgroup\$ – vonludi Aug 11 '16 at 14:18
  • \$\begingroup\$ But i need to actual test if the input is a string, integer or expression in the nested if statement. \$\endgroup\$ – Christian Dean Aug 11 '16 at 18:23
  • \$\begingroup\$ I think I did not state it clearly enough. So, here's a snippet of your code: if (tokens[i] + " " + tokens[i + 1].substr(0, 6) == "output string") { if (tokens[i + 1].substr(0, 6) == "string") ... // omitted ... // also omitted } As you can probably see, the first (surrounding) if already contains the condition of the second (nested) if. You could rewrite the (outer if's) condition to be if(tokens[i] == output && tokens[i+1].substr(0,6) == "string") There does not seem to be a need to concatenate and compare. \$\endgroup\$ – vonludi Aug 12 '16 at 4:31

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.