Reading data from a text file

A

Alex Buell

I have a small text file which consist of the following data:
[[[
-1 -1 -1 1
You are in a small square room.
0 -1 -1 -1
You are in a big round room.
]]]

And the code I've written is as follows:
[[[
Rooms getRoomDataRecords(string roomDataFile)
{
Rooms rooms;

ifstream ifs(roomDataFile.c_str());
if (!ifs.is_open())
{
cout << "Eh? Could not open " << roomDataFile << " for
reading." << endl;
return rooms;
}

while (!ifs.eof())
{
int n, w, e, s;
string line;

ifs >> n >> w >> e >> s;
cout << "1: " << n << " " << w << " " << e << " " <<
s<< endl;
getline(ifs, line);
cout << "2: " << line << endl;

rooms.push_back(Room(n, w, e, s, line));
}

ifs.close();
return rooms;
}
]]]

The trouble is, I can't work out why it goes into an infinite loop
reading the information from the text file!

Can anyone enlighten me as to what I am doing wrong?
 
A

Amal P

Hi,
I can see the below code is working fine...

char* pLine = new char[200];
ifstream DataFile("Data.txt");
while(!DataFile.eof())
{
DataFile.getline( pLine, 100 );
printf("%s\n",pLine);
}

Best regards,
Amal P
 
A

Alex Buell

Hi,
I can see the below code is working fine...

char* pLine = new char[200];
ifstream DataFile("Data.txt");
while(!DataFile.eof())
{
DataFile.getline( pLine, 100 );
printf("%s\n",pLine);
}

Yes, that is absolutely fine but I want to read in the digits from the
first line in the text file into four integer variables.
 
J

Jakob Bieling

Alex Buell said:
I have a small text file which consist of the following data:
[[[
-1 -1 -1 1
You are in a small square room.
0 -1 -1 -1
You are in a big round room.
]]]

And the code I've written is as follows:
[[[
Rooms getRoomDataRecords(string roomDataFile)
{
Rooms rooms;

ifstream ifs(roomDataFile.c_str());
if (!ifs.is_open())
{
cout << "Eh? Could not open " << roomDataFile << " for
reading." << endl;
return rooms;
}

while (!ifs.eof())
{
int n, w, e, s;
string line;

ifs >> n >> w >> e >> s;
cout << "1: " << n << " " << w << " " << e << " " <<
s<< endl;
getline(ifs, line);
cout << "2: " << line << endl;

rooms.push_back(Room(n, w, e, s, line));
}

ifs.close();
return rooms;
}
]]]

The trouble is, I can't work out why it goes into an infinite loop
reading the information from the text file!

Can anyone enlighten me as to what I am doing wrong?

http://www.parashift.com/c++-faq-lite/input-output.html might help
you.

hth
 
A

Alex Buell

Can anyone enlighten me as to what I am doing wrong?

Not to worry, I solved it, there was a hidden bug which made it
harder to track down the problem (attempted to write to memory not
belonging to program) . Once solved, all was well.
 
J

Jerry Coffin

I have a small text file which consist of the following data:
[[[
-1 -1 -1 1
You are in a small square room.
0 -1 -1 -1
You are in a big round room.
]]]

I see elsethread that you've already solved your problem,
but thought I'd add a few points anyway.

Though you don't show its definition, you apparently have
a struct something like this:

struct Room {
int n, w, e, s;
string line;
};

From the looks of the rest of your code, that probably
also defines a ctor, but I'll ignore that since none of
the other code needs it the way I'm writing things. I'd
add a stream extraction operator to read in the data (if
the class/struct makes the data private, it'll probably
need to declare this as a friend):

std::istream &operator>>(std::istream &is, room &r) {
is >> r.n >> r.w >> r.e >> r.s;
std::getline(is, r.line);
return is;
}

Then you can copy your data from the file into a vector
with code something like this (skipping error handling
for the moment):

typedef std::vector<Room> Rooms;

Rooms getRoomDataRecords(std::string roomDataFile) {
Rooms rooms;
ifstream ifs(roomDataFile.c_str());
std::copy(
std::istream_iterator<Room>(ifs),
std::istream_iterator<Room>(),
std::back_inserter(rooms));
return rooms;
}

A couple of basic ideas here: first and foremost, this
keeps all knowledge of the file format associated
directly with the struct/class that represents a record
from the file instead of having that knowledge spread
throughout the code in general.

Second, using the algorithm from the standard library to
copy the data not only clarifies what we're really doing,
but eliminates quite a few kinds of bugs, simply because
we're no longer writing the code that could contain those
bugs.

[ ... ]
The trouble is, I can't work out why it goes into an infinite loop
reading the information from the text file!

One way to avoid that is to avoid writing a loop of your
own at all. Just so you realize, almost any loop of the
form:

while (!x.eof())

is nearly guaranteed to be wrong.
 
A

Alex Buell

A couple of basic ideas here: first and foremost, this
keeps all knowledge of the file format associated
directly with the struct/class that represents a record
from the file instead of having that knowledge spread
throughout the code in general.

Second, using the algorithm from the standard library to
copy the data not only clarifies what we're really doing,
but eliminates quite a few kinds of bugs, simply because
we're no longer writing the code that could contain those
bugs.

Excellent, the code does look a lot better with these changes. I
particularly like the solution using copy. How would that work
with typedef Room* Rooms instead, using new and delete?
One way to avoid that is to avoid writing a loop of your
own at all. Just so you realize, almost any loop of the
form:

while (!x.eof())

is nearly guaranteed to be wrong.

Yeah, I figured that out the other day. Thanks for your input.
 
J

Jerry Coffin

<[email protected]>,
(e-mail address removed) says...

[ ... ]
Excellent, the code does look a lot better with these changes. I
particularly like the solution using copy. How would that work
with typedef Room* Rooms instead, using new and delete?

I'd have to dig the code back up to give specifics, but
in general I'd simply avoid that. A vector is basically a
smart front-end to hold a pointer (and maybe a little
more data with it, like the current allocation size and
amount of that space that's in use) along with code to
manage the allocated memory. Unless you have a fairly
specific reason to do so, you're better off using a
container than a raw pointer.

When/if you do decide that none of the existing
containers will work, your next step would usually be to
write a different container that still supports (at least
most of) the usual interface.

When you need to is soon enough to design your own
storage from the ground up though...
 
A

Alex Buell

I'd have to dig the code back up to give specifics, but
in general I'd simply avoid that. A vector is basically a
smart front-end to hold a pointer (and maybe a little
more data with it, like the current allocation size and
amount of that space that's in use) along with code to
manage the allocated memory. Unless you have a fairly
specific reason to do so, you're better off using a
container than a raw pointer.

I tried doing <Rooms *> but found it too hard to use properly. So I've
stuck with <Rooms>. I just implemented my command handler, but it could
do with improvements, so here it is (all input is assumed to be lower
case but I can always change that to handle upper case):

#include <iostream>
#include <fstream>
#include <sstream>
#include <vector>

#include "command.hpp"
#include "room.hpp"

using namespace std;

Command::Command(string input)
{
stringstream ss(input);
vector<string> tokens;
string buffer;

while (ss >> buffer)
tokens.push_back(buffer);

if (tokens.size() == 0)
{
commandVerb = unknown_verb;
return;
}

commandVerb = parseVerb(tokens.at(0));

if (tokens.size() > 1)
switch (commandVerb)
{
case go:
commandDirection = parseDirection
(tokens.at(1)); break;

default:
break;
}
else
switch (commandVerb)
{
case go:
commandDirection = unknown_direction;
break;

default:
break;
}
}

verb Command::getVerb() const
{
return commandVerb;
}

string Command::getNoun() const
{
return commandNoun;
}

direction Command::getDirection() const
{
return commandDirection;
}

verb Command::parseVerb(string verbString) const
{
if (verbString == "quit")
return quit;

if (verbString == "go")
return go;

return unknown_verb;
}

direction Command::parseDirection(string directionString) const
{
if (directionString == "north")
return north;

if (directionString == "west")
return west;

if (directionString == "east")
return east;

if (directionString == "south")
return south;

return unknown_direction;
}

In main part this is used:

getline(cin, s);

if (s.length() != 0)
{
Command command(s);

switch (command.getVerb())
{
case go:
if (command.getDirection() != unknown_direction)
if (room.canGo(command.getDirection()) !=
BLOCKED)
room = rooms.at (room.canGo
(command.getDirection ()));
else
cout << "Ouch! That way is blocked." <<
endl;
else cout << "Go where?!" << endl;
break;

case quit:
done = true;
break;

default:
cout << "Pardon?!" << endl;
break;
}
 
J

Jerry Coffin

<[email protected]>,
(e-mail address removed) says...

[ ... ]
I tried doing <Rooms *> but found it too hard to use properly. So I've
stuck with <Rooms>. I just implemented my command handler, but it could
do with improvements, so here it is (all input is assumed to be lower
case but I can always change that to handle upper case):

Writing a parser ad hoc like this is more or less asking
for trouble. As long as it stays really simple, it's not
too terrible, but for anything vaguely similar to natural
language, it'll get ugly in a big hurry.

IMO, there are really two reasonable choices. One is to
use a separate generator for the language you're trying
to parse. The other is to write a recursive descent
parser.

Using a typical lex-like lexer generator, your lexer
would look something like this:

quit { return QUIT; }
go { return GO; }
north { return NORTH; }
south { return SOUTH; }
west { return WEST; }
east { return EAST; }

and using a typical yacc-like parser generator the
parser would look something like this:

%token QUIT GO NORTH SOUTH EAST WEST

%union {
enum commands command;
int dir;
};

%type <dir> direction
%type <commands> statements

%%

game: statements;

statements: go_stmt
| quit

go_stmt: GO direction
;

direction: NORTH | SOUTH | EAST | WEST
;

So far, this is really only intended to recognize whether
something is a correctly formatted command or not -- it
doesn't carry out any actions when it does recognize a
command. What you have for actions so far are pretty
trivial though -- basically just check whether you can go
a particular direction before moving that way.

A recursive descent parser is kind of similar in some
ways, but entirely different in others. The idea is
similar to the grammar you use with a parser generator,
but you implement each element with a function (or
functor):

class action {
enum A action;
enum D dir;
};

game() {
action a;
while (a = statement())
execute(a);
}

action statement() {
action ret;
ret.verb = get_token();
if (!is_action(ret.verb))
// error: didn't get a verb

if (ret.verb == GO) {
ret.dir = get_token();
if (!is_dir(ret.dir))
// error didn't get a direction.
}
// ...

This is different from your code in a couple of ways.
First of all, it separates the parsing from the
execution. Second, it's basic structure is built around
the grammar for the language -- the call tree of the
functions directly reflects the abstract syntax tree for
the grammar.

The advantage of using funtors is that you're often
generating a large number of functions that are quite
similar, so much of the commonality can often be moved
into a base class.
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

No members online now.

Forum statistics

Threads
473,744
Messages
2,569,484
Members
44,903
Latest member
orderPeak8CBDGummies

Latest Threads

Top