6

I have a program that take commands from user and it will process different commands differently. For example:

ADD_STUDENT ALEX 5.11 175
ADD_TEACHER MERY 5.4  120 70000
PRINT MERY 
REMOVE ALEX
PRINT TEACHER SALARY
PRINTALL

therefore, I need to examine each line and see what is the input consists of.

Here is my code, but I think I misunderstand the way iss<< work. Can someone give me a suggestion? And tell me how why my code didn't work as I expected?

string line;
while(getline(cin, line))
{
  //some initialization of string, float variable
  std::istringstream iss(line);
  if(iss >> command >> name >> height >> weight)
   ..examine the command is correct(ADD_STUDENT) and then do something..
  else if(iss >> command >> name >> height >> weight >> salary)
   ..examine the command is correct(ADD_TEACHER) and then do something...
  else if(iss >> command >> name)
   ..examine the command is correct(REMOVE) and then do somethin...
}

My thought is that the iss>> first >>second >> third will return true if all arguments are filled and false if not enough arguments. But apparently I am wrong.

user1701840
  • 1,062
  • 3
  • 19
  • 27

5 Answers5

10

Your problem was deliciously underspecified. This always prompts me to supply an overblown example implementation using Boost Spirit.

Note: just don't hand this in as your homework assignment, please.

See it Live on Coliru with the following sample input:

ADD_STUDENT ALEX 5.11 175
ADD_STUDENT PUFF 6 7
ADD_STUDENT MAGIC 7 8
ADD_STUDENT DRAGON 8 9
ADD_TEACHER MERY 5.4  120 70000
PRINT MERY 
ADD_TEACHER DUPLO 5.4  120 140000
PRINTALL  10
REMOVE ALEX
PRINT  TEACHER SALARY
PRINT  MERY PUFF MAGIC DRAGON
REMOVE MERY PUFF MAGIC DRAGON
PRINT  TEACHER SALARY

Full code:

Update When including make_visitor.hpp as shown here you can write the visitor code more elegantly:

auto print_salary = [&] () 
{ 
    for(auto& p : names) 
        boost::apply_visitor(make_visitor(
                    [](Teacher const& v) { std::cout << "Teacher salary: " << v.salary << "\n"; },
                    [](Student const& v) {}), 
                p.second);
};

See adapted example Live on Coliru

#define BOOST_SPIRIT_USE_PHOENIX_V3
#include <boost/spirit/include/qi.hpp>
#include <boost/spirit/include/phoenix.hpp>

namespace qi = boost::spirit::qi;
namespace phx= boost::phoenix;

struct Person
{
    std::string name;
    double height, weight;
    friend std::ostream& operator<<(std::ostream& os, Person const& s) {
        return os << "Person { name:" << s.name << ", height:" << s.height << ", weight:" << s.weight << " }";
    }
};

struct Student : Person
{
    Student() = default;
    Student(std::string n, double h, double w) : Person {n,h,w} {}
};

struct Teacher : Person
{
    Teacher() = default;
    Teacher(std::string n, double h, double w, double s) : Person {n,h,w}, salary(s) {}
    double salary;
};

int main()
{
    std::stringstream ss;
    ss << std::cin.rdbuf();

    std::map<std::string, boost::variant<Student, Teacher> > names;

    using namespace qi;
    auto add_student  = phx::ref(names)[_1] = phx::construct<Student>(_1, _2, _3);
    auto add_teacher  = phx::ref(names)[_1] = phx::construct<Teacher>(_1, _2, _3, _4);
    auto remove       = phx::erase(phx::ref(names), _1);
    auto print_all    = [&] (int i) { for(auto& p : names) { std::cout << p.second << "\n"; if (--i==0) break; } };
    auto print_salary = [&] () 
    { 
        struct _ : boost::static_visitor<> {
            void operator()(Teacher const& v) const { std::cout << "Teacher salary: " << v.salary << "\n"; }
            void operator()(Student const& v) const { }
        } v_;
        for(auto& p : names) boost::apply_visitor(v_, p.second);
    };

    auto name_ = as_string[lexeme[+graph]];

    if (phrase_parse(begin(ss.str()), end(ss.str()), 
                (
                     ("ADD_STUDENT" >> name_ >> double_ >> double_)            [ add_student ]
                   | ("ADD_TEACHER" >> name_ >> double_ >> double_ >> double_) [ add_teacher ]
                   | (eps >> "PRINT" >> "TEACHER" >> "SALARY")                 [ print_salary ]
                   | ("PRINTALL" >> int_)      [ phx::bind(print_all, _1) ]
                   | ("PRINT"  >> +name_       [ std::cout << phx::ref(names)[_1] << std::endl ])
                   | ("REMOVE" >> +name_       [ remove ])
                ) % +eol,
                qi::blank))
    {
        std::cout << "Success";
    }
    else
    {
        std::cout << "Parse failure";
    }
}

Output:

Person { name:MERY, height:5.4, weight:120 }
Person { name:ALEX, height:5.11, weight:175 }
Person { name:DRAGON, height:8, weight:9 }
Person { name:DUPLO, height:5.4, weight:120 }
Person { name:MAGIC, height:7, weight:8 }
Person { name:MERY, height:5.4, weight:120 }
Person { name:PUFF, height:6, weight:7 }
Teacher salary: 140000
Teacher salary: 70000
Person { name:MERY, height:5.4, weight:120 }
Person { name:PUFF, height:6, weight:7 }
Person { name:MAGIC, height:7, weight:8 }
Person { name:DRAGON, height:8, weight:9 }
Teacher salary: 140000
Success
sehe
  • 374,641
  • 47
  • 450
  • 633
  • 1
    @kbok Thank you. Although I intend this code rather jokingly. It is _certainly_ the most complete answer. By a mile. Or seven. :) – sehe Jul 22 '13 at 19:12
7

Do it like so:

iss >> command;
if (!iss)
    cout << "error: can not read command\n";
else if (command == "ADD_STUDENT")  
    iss >> name >> height >> weight;
else if (command == "ADD_TEACHER")  
    iss >> name >> height >> weight >> salary;
else if ...
Leonid Volnitsky
  • 8,854
  • 5
  • 38
  • 53
  • The == operator is illegal tho – user1701840 Jul 21 '13 at 01:29
  • 6
    It is not illegal for `std::string`. 2nd argument (c-string) is converted to `std::string`. – Leonid Volnitsky Jul 21 '13 at 01:40
  • @user1701840 Provided that `command` is a `std::string` then it's legal. – Felix Glas Jul 21 '13 at 01:43
  • 2
    @LeonidVolnitsky I don't think the c-string is converted, the operator simply has an overload `bool operator== (std::string lhs, const char* rhs)`. – Felix Glas Jul 21 '13 at 01:46
  • @snipes83 -- yep, special overload for c-strings. – Leonid Volnitsky Jul 21 '13 at 01:49
  • Well, this works fine for perfect input and fixed numbers of arguments. You need another bit of code as well (which I don't have lying around) to discard all input up to and including the terminating `'\n'`. `cin >> endl;` doesn't do it. – Mike DeSimone Jul 21 '13 at 01:53
  • Another thing is that `>>` will eat a line end in search of the next token, so if you provide too *few* inputs, you'll wind up grabbing part of the next line. This might be acceptable, but you should make sure. – Mike DeSimone Jul 21 '13 at 01:55
  • @MikeDeSimone: Code of OP, does not read from `cin`, it reads from `stringstream`. Lines are correctly read with `getline`. It would be nice to check if fields are read correctly though. – Leonid Volnitsky Jul 21 '13 at 02:04
  • your previous answer works as well. using iss && command == "ADD_STUDENT" – user1701840 Jul 21 '13 at 02:18
  • @user1701840 -- Yes, it was correct but it was doing extraneous check for state of `iss` value in every `else if`. – Leonid Volnitsky Jul 21 '13 at 02:26
  • 1
    Usually, when `!iss` triggers, it's end-of-file. Make sure it doesn't get stuck in an infinite loop. – Mike DeSimone Jul 21 '13 at 16:00
  • @MikeDeSimone: `Usually, when !iss triggers, it's end-of-file` -- this is incorrect. `!iss` is equivalent to `!iss.good()`, not to `iss.eof()`. And EOF is checked in OP's: `while(getline(cin, line))` – Leonid Volnitsky Jul 21 '13 at 16:37
  • @LeonidVolnitsky: And Mike is correct that the end-of-file is the most common reason for the stream failing. But that's not even necessary to his point. As long as there is *any* way to cause an infinite loop, the program should be rethought. But you're right, there is no infinite loop. – Ben Voigt Jul 22 '13 at 01:49
5

Your problem is that using the >> operator reads and clears a token from the stream.

if(iss >> command >> name >> height >> weight)

This (above) tries to read 4 tokens from the stream, and for every successful read, it clears the read data from the stream.

else if(iss >> command >> name >> height >> weight >> salary)

When you get to this (above) it means that some token could not be read and cast into the appropriate type, however it is likely that at least the command token is already stripped from the stream.

Felix Glas
  • 15,065
  • 7
  • 53
  • 82
2

Well, it's too late for much chance at upvotes, but you guys got me thinking about this...

For robustness, you could split parsing into two stages: the first stage gets lines, and the second stage takes a line and does something with it.

For the first stage, you can use the getline:

#include <string>
#include <sstream>

void ParseLines(std::istream& source)
{
    while(source)
    {
        // Get a line from the source.
        std::string inputLine;
        std::getline(source, inputLine);

        // Make a stream out of it.
        std::istringstream inputStream(inputLine);
        std::string command;
        inputStream >> command;
        if(inputStream) // Empty or bad line: skip
            HandleCommand(command, inputStream);
    }
}

The second stage handles the command. It could be something direct like this:

void HandleCommand(const std::string& command, std::istringstream& params)
{
    if(command == "ADD_STUDENT")
    {
        float someFloat;
        int someInt;
        params >> someFloat >> someInt;
        // add the student.
    }
    // etc.
}

But I have no shame and would implement a Factory paradigm:

#include <map>

typedef void (*CommandHandler)(const std::string&, std::istringstream&);
typedef std::map<std::string, CommandHandler> CommandTable;

static CommandTable gCommands; // Yep. A global. Refactor however you see fit.

void HandleCommand(const std::string& command, std::istringstream& params)
{
    CommandTable::iterator handler = gCommands.find(command);
    if(handler == gCommands.end())
    {
        // Handle "command not found" error.
        return;
    }

    (*(handler->second))(command, params);
}

void AddStudent(const std::string& command, std::istringstream& params)
{
    float someFloat;
    int someInt;
    params >> someFloat >> someInt;
    // add the student.
}

// Other command handling functions here...

void RegisterCommands()
// Call this once prior to parsing anything,
// usually one of the first things in main().
{
    gCommands["ADD_STUDENT"] = &AddStudent;
    // ... other commands follow...
)

Haven't tested any of this, but it should be mostly there. Note any bugs in the comments.

P.S. This is highly inefficient and will run slower than a properly designed command parser, however, it should be good enough for most jobs.

Mike DeSimone
  • 41,631
  • 10
  • 72
  • 96
  • looks like a good idea than throwing everything inside the main! Thanks! – user1701840 Jul 21 '13 at 15:24
  • 1
    Hohum. My answer weighs **[almost exactly the same number of LoC](http://coliru.stacked-crooked.com/view?id=c64e825675981f1787285a2c1893ab94-acc21326bddb78ffadef615690276e19)** as your answer shows. Only, mine doesn't do shortcuts: it actually implements the commands and types, storage and lookup :/ Oh, and I posted it even later. (I'm a sucker for this kind of task like you, apparently :)) – sehe Jul 22 '13 at 02:02
  • 1
    Just no pleasing some people. I used function pointers to keep the answer simple, *and* I deliberately restricted to C++03 or so because I have no idea how old this guy's compiler is. Like I said, refactor any way you feel like because the right refactoring *depends on how the rest of the code is organized, and none of that was posted.* C++11 provides a lot better options. Also, I was deliberately *avoiding* Boost Spirit so as not to confuse the poor OP's mind; they're just trying to use `istream`'s `operator >>`. BTW, thanks for the downvote in spite. – Mike DeSimone Jul 22 '13 at 04:23
  • 1
    What spite? Your answer is bad. The OP didn't specify C++03 and even if he did, there is tr1::function and boost::function. As for the global variables, posting an answer using globals is bad. You could at least take a parameter or something. – Puppy Jul 22 '13 at 11:44
1

You could technically tokenize the entire input line, but that seems a little too far out from your level. If you did want to go into it, there is a nice page and tutorial here that will help you use strtok().

If you do not want to go that method, you can individually parse through your list of commands. Say you have read into a string named "command".

if (command == "ADD_STUDENT")
{
    int weight, height, otherfield;
    cout << ">" << flush;
    cin >> weight >> height >> otherfield;
    //do something, like add them to the database
}

That seems like your best bet, although it is a lot of coding, it is probably easier for you to accomplish. You could get really into it and use format strings like this:

scanf("%s, %s %d, %f", lastname, firstname, age, height);

This way, the input would look like this:

ADD_STUDENT Doe, John 30, 5.6
phyrrus9
  • 1,441
  • 11
  • 26
  • You don't need to resort to `scanf` and `strtok` to tokenize lines. See http://stackoverflow.com/questions/53849/how-do-i-tokenize-a-string-in-c – Mike DeSimone Jul 21 '13 at 01:58
  • @Mike it really matters on what type of string you're using. I never use std::string, so .split() wont work for me. – phyrrus9 Jul 21 '13 at 02:01