Google Groups no longer supports new Usenet posts or subscriptions. Historical content remains viewable.
Dismiss

file open in C++

1 view
Skip to first unread message

arunix

unread,
Nov 22, 2009, 12:39:39 AM11/22/09
to
Hello all,
here is a code to open the file....
how can i be better in this...

#include<iostream>
#include<string>
#include<fstream>

int main()

{
std :: string line;
char f_nme[20];
std :: ifstream myfile;
std :: cout <<"Please enter File name ";
std :: cin >> f_nme;
myfile.open(f_nme);
if(myfile.is_open())
{
while(!myfile.eof())
{
getline(myfile,line);
std :: cout<< line << std:: endl;
}
myfile.close();
}
else
{
std :: cout <<"Enable to Open The File " << std :: endl;
}
return 0;
}

Thanks

Saeed Amrollahi

unread,
Nov 22, 2009, 1:43:55 AM11/22/09
to

Hi
1. You can use std::string as filename. You take advantage of strings
over char array:
string f_name;
because for historical reosons, the stream file interface works with
char*,
we have to use string::c_str()
myfile.open(f_nme.c_str());
2. You can use operator! instread of is_open. At the moment I don't
know the
advantage, but my be it is more general, because operator! check the
state of
file against bad state (all corruption states):
if (!my_file) {
// error message or exception handling
}
else {
// read the file
}
3. You don't need to close file stream, because at the end of scope
the stream dtor is called implicitly.

Regards,
-- Saeed Amrollahi

arunix

unread,
Nov 22, 2009, 3:26:53 AM11/22/09
to
Thanks for the reply

> we have to use string::c_str()
>   myfile.open(f_nme.c_str());

Great
i never thought about it because i thought i need only few character
for the file name..

Saeed Amrollahi

unread,
Nov 22, 2009, 3:50:51 AM11/22/09
to

I am happy to help you.
As I noted, for historical reasons only, the file stream constructor
uses the char* for file name.
Bjarne Stroustrup proposed uniform use of std::string in all C-style
string interfaces. May be in next revision of C++ (I mean C++1X) will
be
considered.

My two cents ...
-- Saeed Amrollahi

Rolf Magnus

unread,
Nov 22, 2009, 11:32:32 AM11/22/09
to
arunix wrote:

Your loop is incorrect. For one, it will give you the last line twice, and
if some error happens, it will become an endless loop. It's a classical
error and thus is described in chapter 15 of the FAQ to this newsgroup.


Victor Bazarov

unread,
Nov 22, 2009, 11:43:28 AM11/22/09
to

Just a note, the _next_ version of the language is called "C++0x". You
might want to explain what you mean by "C++1X", perhaps the version
after next?

> be
> considered.
>
> My two cents ...
> -- Saeed Amrollahi

V
--
Please remove capital 'A's when replying by e-mail
I do not respond to top-posted replies, please don't ask

Rolf Magnus

unread,
Nov 22, 2009, 12:30:40 PM11/22/09
to
Victor Bazarov wrote:

> Saeed Amrollahi wrote:
>> On Nov 22, 11:26 am, arunix <arru...@gmail.com> wrote:
>>> Thanks for the reply
>>>
>>>> we have to use string::c_str()
>>>> myfile.open(f_nme.c_str());
>>> Great
>>> i never thought about it because i thought i need only few character
>>> for the file name..
>>
>> I am happy to help you.
>> As I noted, for historical reasons only, the file stream constructor
>> uses the char* for file name.
>> Bjarne Stroustrup proposed uniform use of std::string in all C-style
>> string interfaces. May be in next revision of C++ (I mean C++1X) will
>
> Just a note, the _next_ version of the language is called "C++0x". You
> might want to explain what you mean by "C++1X", perhaps the version
> after next?

Well, it's inofficially called "C++0x" because it was supposed to be
released in 200x, but that won't happen. So renaming it to C++1x seems
reasonable to me.

Juha Nieminen

unread,
Nov 22, 2009, 1:12:41 PM11/22/09
to
arunix wrote:
> std :: string line;
> char f_nme[20];
> std :: ifstream myfile;
> std :: cout <<"Please enter File name ";
> std :: cin >> f_nme;
> myfile.open(f_nme);

Safer and easier:

std::string fileName;
std::cin >> fileName;
std::ifstream myFile(fileName.c_str());

> if(myfile.is_open())

Since you can retrieve the *reason* why opening the file failed, it's
a good idea to tell the user:

if(!myFile)
{
std::cerr << "Could not open ";
std::perror(myFile.c_str());
return 1;
}

> while(!myfile.eof())
> {
> getline(myfile,line);
> std :: cout<< line << std:: endl;
> }

Infinite loop if an error happens during reading, and you are making
the check in the wrong place. The correct way is:

while(true)
{
std::getline(myFile, line);
if(!myFile) break; // Check success before printing anything


std::cout << line << std::endl:
}

> myfile.close();

Usually no need to explicitly close it, but doesn't hurt either.

Pete Becker

unread,
Nov 22, 2009, 2:30:37 PM11/22/09
to

Reasonable or not, it's still C++0x.

--
Pete
Roundhouse Consulting, Ltd. (www.versatilecoding.com) Author of
"The Standard C++ Library Extensions: a Tutorial and Reference"
(www.petebecker.com/tr1book)

Rolf Magnus

unread,
Nov 23, 2009, 12:57:44 AM11/23/09
to
Juha Nieminen wrote:

> if(!myFile)
> {
> std::cerr << "Could not open ";
> std::perror(myFile.c_str());

Is std::ifstream guaranteed to set errno in case of an error?

> return 1;
> }
>
>> while(!myfile.eof())
>> {
>> getline(myfile,line);
>> std :: cout<< line << std:: endl;
>> }
>
> Infinite loop if an error happens during reading, and you are making
> the check in the wrong place. The correct way is:
>
> while(true)
> {
> std::getline(myFile, line);
> if(!myFile) break; // Check success before printing anything
> std::cout << line << std::endl:
> }

What's wrong with the usual

while(std::getline(myFile, line))
{


std::cout << line << std::endl;
}

I find that more readable, because it doesn't hide the loop's exit condition
somewhere in the middle, but rather puts it where it belongs.

Saeed Amrollahi

unread,
Nov 23, 2009, 1:05:52 AM11/23/09
to

Hi Victor

By next revision of C++, I mean the next revision after C++0x,
and it is called inofficially C++1x.
It is obvious we won't have C++0x at 2009, and hopefully x is A or may
be B.
A lot of committee members especially Bjarne Stroustrup still uses C+
+0x.
The C++ standard committee likes the -long- 8-10 years period for a
revision
becomes shorter.

sorry for misunderestanding
-- Saeed Amrollahi

James Kanze

unread,
Nov 23, 2009, 6:10:40 AM11/23/09
to
On Nov 23, 5:57 am, Rolf Magnus <ramag...@t-online.de> wrote:
> Juha Nieminen wrote:
> > if(!myFile)
> > {
> > std::cerr << "Could not open ";
> > std::perror(myFile.c_str());

> Is std::ifstream guaranteed to set errno in case of an error?

Formally, I don't think so, but in practice, it probably will.

> > return 1;
> > }

> >> while(!myfile.eof())
> >> {
> >> getline(myfile,line);
> >> std :: cout<< line << std:: endl;
> >> }

> > Infinite loop if an error happens during reading, and you
> > are making the check in the wrong place. The correct way is:

> > while(true)
> > {
> > std::getline(myFile, line);
> > if(!myFile) break; // Check success before printing anything
> > std::cout << line << std::endl:
> > }

> What's wrong with the usual

> while(std::getline(myFile, line))
> {
> std::cout << line << std::endl;
> }

> I find that more readable, because it doesn't hide the loop's
> exit condition somewhere in the middle, but rather puts it
> where it belongs.

Because it modifies state in a condition?

In fact, the form you suggest is so ubiquious that you should
use it exclusively, unless there is a very strong reason not to.
Anything else, and the reader will ask why.

> >> myfile.close();

> > Usually no need to explicitly close it, but doesn't hurt either.

Note that that's only true for input (where you've already
checked the state after reading). For output, you need an
explicit close, because you have to check the state of the
stream after the close.

--
James Kanze

red floyd

unread,
Nov 23, 2009, 1:08:29 PM11/23/09
to
On Nov 23, 3:10 am, James Kanze <james.ka...@gmail.com> wrote:
> On Nov 23, 5:57 am, Rolf Magnus <ramag...@t-online.de> wrote:
>
> > Juha Nieminen wrote:
> > >     if(!myFile)
> > >     {
> > >         std::cerr << "Could not open ";
> > >         std::perror(myFile.c_str());
> > Is std::ifstream guaranteed to set errno in case of an error?
>
> Formally, I don't think so, but in practice, it probably will.

Is this corrected in C++0x?

White Wolf

unread,
Nov 23, 2009, 7:57:21 PM11/23/09
to

Corrected?

--
BR, WW

red floyd

unread,
Nov 23, 2009, 9:31:09 PM11/23/09
to

Corrected may be the wrong word, but it seems that if fopen() and
friends from <cstdio> can set errno, fstream and its children should
be able to set errno. If nothing else, it would make error reporting
code standard-compliant.

Juha Nieminen

unread,
Nov 24, 2009, 11:18:33 AM11/24/09
to
Rolf Magnus wrote:
> What's wrong with the usual
>
> while(std::getline(myFile, line))
> {
> std::cout << line << std::endl;
> }

Not much, I suppose. I'm just accustomed to writing this kind of code
(because of the kind of programs I have to write):

while(true)
{
is >> keyword;
if(!is || keyword == "end") break;
...
}

I suppose you *could* write it like:

while((is >> keyword) && keyword != "end")

but that would start being a bit obfuscated, IMO.

James Kanze

unread,
Nov 25, 2009, 4:22:42 AM11/25/09
to

It's a lot clearer than what you write. Basically, you start by
saying "loop forever", and then somewhere more less hidden, you
later say "fooled you, didn't I---it's not forever".

--
James Kanze

Juha Nieminen

unread,
Nov 25, 2009, 4:32:58 PM11/25/09
to
James Kanze wrote:
> It's a lot clearer than what you write. Basically, you start by
> saying "loop forever", and then somewhere more less hidden, you
> later say "fooled you, didn't I---it's not forever".

I think it's pretty obvious that if you are parsing a file, you won't
loop forever (especially since the 'break' that ends the loop is in the
second line of the loop body, and nowhere else).

Of course if you want to be pedantic about it, you could use a boolean
variable like this:

bool done = false;
while(!done)
{
is >> keyword;
if(!is || keyword == "end") done = true;
else
...
}


but I don't really see that being any clearer in the end.

James Kanze

unread,
Nov 26, 2009, 4:25:10 AM11/26/09
to
On Nov 25, 9:32 pm, Juha Nieminen <nos...@thanks.invalid> wrote:
> James Kanze wrote:
> > It's a lot clearer than what you write. Basically, you
> > start by saying "loop forever", and then somewhere more less
> > hidden, you later say "fooled you, didn't I---it's not
> > forever".

> I think it's pretty obvious that if you are parsing a file,
> you won't loop forever (especially since the 'break' that ends
> the loop is in the second line of the loop body, and nowhere
> else).

In very small cases, it doesn't really make a difference; in
fact, I'd argue that if it did make a significant difference,
you're loop (and in fact your complete function) is too complex,
and needs to be broken up into smaller pieces. But in general,
it's considerably cleaner not to put exit conditions just
anywhere, in a way which requires people to look for them.

(To be frank, in so far as there is just one, the situation
could be improved by different formatting: indenting the if
which controls the break at the same level as the loop
construct, for example, because it really is part of the loop
construct, and not of what is being done in the loop. But I've
never seen this style used, and most people who use break to get
out of a loop have no hesitations about using it in a nested if,
or some other construct which is not immediately visible.)

> Of course if you want to be pedantic about it, you could use a
> boolean variable like this:

> bool done = false;
> while(!done)
> {
> is >> keyword;
> if(!is || keyword == "end") done = true;
> else
> ...
> }

> but I don't really see that being any clearer in the end.

Perhaps if you'd name the variable more clearly, say
"end_statement_seen". But even as above, it's slightly clearer;
it's clear that the loop does end, although it's still a little
vague about what "done" means.

--
James Kanze

dragan

unread,
Dec 5, 2009, 1:46:53 AM12/5/09
to
Saeed Amrollahi wrote:

> Bjarne Stroustrup proposed uniform use of std::string in all C-style
> string interfaces.

Tell me if I have this right:

One key point of pushing C++ features out to libraries was to provide
mechanism and avoid policy. Then someone decides it's a good idea to take
one constituent of a library and weave it throughout the library. Now then,
a programmer must choose yeh or nay on the whole package just because he
doesn't use std::string. How is this good? :/

0 new messages