ansaurus

Question

Simple C++ Writing to File: Nothing Gets Written! Should be easy, but I'm stumpted.

Answer 1

+3 A:

[Edit] The need for explicit flush() calls was something I encountered with old, broken, pre-standard compilers like MSVC 6 and earlier over a decade ago. It is probably not necessary any more and apparently was a workaround for problematic library implementations.

I've tried out the code (removing the section to output messages and renaming the output file name to something on my system). The output was correct.

It might be worth trying to write to a local file just to see what happens. You already check for failure to open the file stream for output but there appears to be something odd going on for your case.

[Edit: I'm probably going to get down-voted for being nosy but...] Your C style habits of defining all variables at the top of a given scope are obsolete. Consider defining variables no sooner than where they can be properly initialized and with a more limited scope. It may not be a big deal now, but if you ever work on a C system of any reasonable scale and encounter your first uninitialized variable bugs, you're going to start despising it. Everyone's entitled to their personal preferences, but a style that promotes bugs is an inferior one, objectively speaking.

2010-06-29 10:15:40

That's nonsense. Streams are automatically closed on destruction and are automatically flushed on close.

sbi 2010-06-29 10:39:59

I fully agree on the style advice, though.

sbi 2010-06-29 10:45:26

@sbi You are right. I was basing this on working with pre-standard compilers like MSVC 6. In the past, I found explicit calls to flush necesssary but it certainly appears not to be the case anymore.

2010-06-29 22:28:30

BTW I consider having variables at the top of a scope to be much more readable coding practice.

Paul Nathan 2010-06-29 22:41:48

@Paul It's a disastrous coding practice. Unavoidable in C, but fortunately avoidable in C++. There's plenty of literature from higher authorities than me to back this up. You'll change your mind about it if you ever work in a large-scale C system filled with millions of lines of code this way and riddled with uninitialized variable bugs, mem leaks, and other malpractices associated with this style that could have been easily avoided merely by defining variables where they can be sensibly initialized.

2010-06-29 22:59:06

@stinky472: Upvoted for the variable declaration thing. Variables should have the least possible scope.

DeadMG 2010-06-29 23:08:55

@stinky472: I've found min-scope decls to be a terrific pain to maintain and track. I know "top-of-scope" variables aren't cool and modern, but they really work well for me.

Paul Nathan 2010-06-30 00:10:12

@Paul it's not about being cool or modern, it's about avoiding problems. Individuals can get away with unsafe code, but for large teams, safety becomes increasingly important. You might be so great that you can get away with that style and never have uninitialized variables, but when the system has an uninitialized variable problem that occurs only in release production builds (something hard to reproduce and never shows up in debug builds), now your code has to be reviewed as a major suspect and it just adds more time to everyone's debugging work to try to see if your code is the source.

2010-06-30 04:34:59

@Paul It may not have been your fault if such a thing happened, but your code ends up being a major suspect when you write it that way and then it has to be reviewed. It's like taking a dump all over the evidence in an investigation: it just slows everything down. If you define all variables when they are initialized, no one has to bother investigating your code.

2010-06-30 04:39:33

Also have you ever tried using a debugger? I don't mean to sound disrespectful with that question, but I think most would agree that declaring/defining variables when they can be meaningfully initialized makes code a lot easier to debug. For a start, you don't to have to look at locals which aren't being used at with garbage memory until they can be sensibly used, at which point they'll have meaningful values. It's terribly confusing debugging large C functions which define all variables at the top and trying to figure out whether 'A' having garbage is because 'A' doesn't have to be used yet.

2010-06-30 04:41:56

@stinky. having variables 'randomly' show up all over the place is much harder to track than having a block of variables in one easy-to-access place. Further, if you are using a quasi-decent compiler(or a static analysis tool????), it should warn you of unintialized variables. I'm sorry, but I don't find your evidences persuasive at all. You need to cite your unnamed 'higher authorities', I think.

Paul Nathan 2010-06-30 06:28:46

@Paul one would be Herb Sutter of the ISO C++ committee. He cites this multiple times, but Modern C++ Coding Standards is a good one to check out. Stroustrup also goes into this in The C++ Programming Language (why do you think he allowed it so we don't have to define all variables at the top of a block?).

2010-06-30 06:37:11

Finally, compilers do a poor job of detecting uninitialized variables. Consider this case: int* p; f( Compilers don't warn in this case because this is common behavior in C and (unfortunately) sometimes even C++. If it was as easy as spotting compiler warnings, we would not have these kinds of cases in large-scale C systems but ask anyone who has worked on such a system and they will surely tell you horror stories about uninitialized variables that the compiler didn't catch (one of the most common sources of release-only bugs other than timing issues like race conditions).

2010-06-30 06:39:53

"In particular, older versions of C before [C99] required variables to be defined only at the beginning of a scope; this style is obsolete in C++. A serious problem[...] is that [...] you often don't yet have enough information to initialize variables with pertinent information. This leaves you with two choices: either initialize with some default blank value[...], which is usually wasteful and can lead to errors if the variable ends up being used before it has a useful state, or leave them uninitialized, which is dangerous.[...] The cure is simple: Define each variable as locally as you can"

2010-06-30 06:44:51

[Sutter, Herb. C++ Coding Standards, Ch. 18] Apologies for skipping sections of it, but it's hard to work with these character restrictions for comments. I can't recommend this book enough for working on a team. The book does not preach style; the title made my stomach churn as well when it was recommended to me. Instead it teaches guidelines on how to write robust and efficient code for large-scale systems.

2010-06-30 06:46:51

@stinky472: I agree with your comment about `fflush()`, which I sometimes need to explicitly call before `fclose()` on HPUX 10.20, despite what the man page says. And certain ATC systems depend on "old, broken" OSs. But I think I disagree about variable thing. Anyway, +1 for `fflush()`.

Joseph Quinsey 2010-06-30 19:46:46

@Paul: I would wager that any decent introductory C++ book will mention that variables should be defined as late as possbile. (And if it doesn't, it isn't decent enough.) ISTR having first read it from Stroustrup, which back then likely would have been the 1st or 2nd edition of TC++PL. And, yes, all the big names will tell you the same. The C++ FAQ has a topic on it, too: http://www.parashift.com/c++-faq-lite/coding-standards.html#faq-27.7: "Locals should be declared near their first use." You need to change your habit, since it's bad. (And not only for performance reasons as the FAQ says.)

sbi 2010-06-30 23:36:18

Answer 2

A:

To check for a stream that was not opened, use:

if (myfile.bad())

The rest looks good, its just that the file path is wrong, or the file cannot be created.

gbjbaanb 2010-06-29 10:17:47

The `myfile.bad()` check is included in `!myfile`.

sbi 2010-06-29 10:38:26

Answer 3

+1 A:

Your code runs fine on my machine using VS 2005. This expression was giving an error :

myMessages[i] != ""

Converted to

myMessages[i].empty()

which is better because you are using the string library and method is there to check if a string is empty or not, why not use it.

DumbCoder 2010-06-29 10:53:12

I reckon that'd probably actually be `!myMessages[i].empty()`.

Mac 2010-06-29 20:05:55

Answer 4

+3 A:

A few tests to try:

What happens if you send your output to stdout instead of to a file?
What happens if you use a different file path, like "C:\\out.htm"?
What happens if you run this when the file doesn't exist? What about if you manually create the (empty) output file before running the program?
What happens if you simplify the program down to just a simple open, myfile << "test";, close?
What happens if you try to run the abridged version of the program using C-style file I/O (fopen, fprintf, fclose) instead of streams?

bta 2010-06-29 22:49:34

+1 great suggestions.

2010-06-30 03:43:49

ansaurus

tags:

views:

answers:

Simple C++ Writing to File: Nothing Gets Written! Should be easy, but I'm stumpted.

related questions