472,125 Members | 1,477 Online
Bytes | Software Development & Data Engineering Community
Post +

Home Posts Topics Members FAQ

Join Bytes to post your question to a community of 472,125 software developers and data experts.

Reading data from a text file

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.munted.org.uk

"Honestly, what can I possibly say to get you into my bed?" - Anon.
Mar 17 '06 #1
9 2639
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

Mar 17 '06 #2
On 17 Mar 2006 03:31:48 -0800 "Amal P" <en********@gmail.com> waved a
wand and this message magically appeared:
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.

--
http://www.munted.org.uk

"Honestly, what can I possibly say to get you into my bed?" - Anon.
Mar 17 '06 #3
Alex Buell <al********@munted.org.uk> wrote:
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
--
jb

(reply address in rot13, unscramble first)
Mar 17 '06 #4
On Fri, 17 Mar 2006 09:55:59 +0000 Alex Buell
<al********@munted.org.uk> waved a wand and this message magically
appeared:
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.

--
http://www.munted.org.uk

"Honestly, what can I possibly say to get you into my bed?" - Anon.
Mar 17 '06 #5
In article
<20********************************@munted.org.uk> ,
al********@munted.org.uk says...
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.

--
Later,
Jerry.

The universe is a figment of its own imagination.
Mar 20 '06 #6
On Sun, 19 Mar 2006 21:40:20 -0700 Jerry Coffin <jc*****@taeus.com>
waved a wand and this message magically appeared:
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.

--
http://www.munted.org.uk

"Honestly, what can I possibly say to get you into my bed?" - Anon.
Mar 20 '06 #7
In article
<20********************************@munted.org.uk> ,
al********@munted.org.uk 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...

--
Later,
Jerry.

The universe is a figment of its own imagination.
Mar 21 '06 #8
On Mon, 20 Mar 2006 23:39:20 -0700 Jerry Coffin <jc*****@taeus.com>
waved a wand and this message magically appeared:
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.


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;
}

--
http://www.munted.org.uk

"Honestly, what can I possibly say to get you into my bed?" - Anon.
Mar 21 '06 #9
In article
<20********************************@munted.org.uk> ,
al********@munted.org.uk 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.

--
Later,
Jerry.

The universe is a figment of its own imagination.
Mar 22 '06 #10

This discussion thread is closed

Replies have been disabled for this discussion.

Similar topics

1 post views Thread by fabrice | last post: by
7 posts views Thread by John Dann | last post: by
4 posts views Thread by Amit Maheshwari | last post: by
6 posts views Thread by arne.muller | last post: by
11 posts views Thread by Freddy Coal | last post: by
13 posts views Thread by swetha | last post: by
21 posts views Thread by Stephen.Schoenberger | last post: by
reply views Thread by leo001 | last post: by

By using Bytes.com and it's services, you agree to our Privacy Policy and Terms of Use.

To disable or enable advertisements and analytics tracking please visit the manage ads & tracking page.