3

Why does the following code not work

#include <iostream>
#include <fstream>
#include <stdio.h>
#include <string.h>
#include <stdlib.h>
using namespace std;

int main(){
    string data;
    int i=0;

    while(i <= 5){
      i++;
      data += i;
      data += "\n";
    }

    ofstream myfile;
    myfile.open ("data.txt");
    myfile << data;
    myfile.close();
}

It should append a number then newline and write it to a file (that doesn't yet exist).

The file should look like this...

1
2
3
4
5

What is wrong with the code?

James
  • 2,454
  • 1
  • 22
  • 22
Mark Lalor
  • 7,820
  • 18
  • 67
  • 106
  • Well, what do you get out of it so far? – James Aug 24 '10 at 16:29
  • The program will not compile. Where is main()? – riwalk Aug 24 '10 at 16:32
  • 2
    `` should be ``, `` should be ``, and `` should be ``. And if you're going by a book, you need a new one. :) Try Accelerated C++, and see here: http://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list – John Dibling Aug 24 '10 at 16:42

5 Answers5

11

Why do you not use operator<<?

ofstream myfile;
myfile.open ("data.txt");
for ( int i = 1; i <= 5; ++i )
  myfile << i << "\n";
myfile.close();
Kirill V. Lyadvinsky
  • 97,037
  • 24
  • 136
  • 212
  • +1: also: if the intermediate string representation is important, use a stringstream in the place of the ofstream to have access to the string representation. – rubenvb Aug 24 '10 at 16:37
  • +1 This is the most correct answer, unless there is a reason to build the string (i.e. Manipulations done before tossing it in the file) then there is no reason for it to be there. – James Aug 24 '10 at 16:39
2

Multiple issues with your code. First of all, you're #include-ing several deprecated headers, including <stdio.h> which should be <iostream>, <string.h> which should be <string>, and <stdlib.h> which should be <cstdlib>.

As for your specific question, it is doing exactly what you asked it to do. Problem is, you didn't ask it to do what you wanted it to do. In your code data += i; you are saying "append the binary value i to this string" which your code dutifully does. If you open your resulting text file in a binary-capable text editor, you'll find it is the binary data you inserted.

What you wanted to do was convert the integers to their string representations, and append that to the text file. A simple way do this in a C++ish way is to use stringstream, as so:

#include <iostream>
#include <fstream>
#include <iostream>
#include <string>
#include <sstream>
#include <cstdlib>
using namespace std;

int main(){
    int i=0;

        stringstream ss;
    while(i <= 5){
      i++;
            ss << i << endl;
    }

    ofstream myfile;
    myfile.open ("data.txt");
    myfile << ss.str();
    myfile.close();
}
John Dibling
  • 99,718
  • 31
  • 186
  • 324
1

The main problem that i see is that you are building off of the string data by adding integers to it.
What you actually want is the character version of the integer which would require something like this:

while(i <= 5){
  i++;
  data += char(i+48);
  data += "\n";
}

What this does is add the offset (48) from decimal representation to the ASCII representation of the number.
EDIT: Tested this, depending if you want to print up to 6 or not you're also gonna wanna replace the while(i <= 5) with a while(i < 5) due to the way you set up the inside of the while loop.

James
  • 2,454
  • 1
  • 22
  • 22
  • IMHO, the assignment would read better as `data += '0' + i;`. In this manner there is no need for the `char` cast, as the variable `i` is converted by the the compiler. – Thomas Matthews Aug 24 '10 at 16:35
  • Works very well, that was simple enough – Mark Lalor Aug 24 '10 at 16:37
  • @ThomasMatthews it is better to read that way, however i believe seeing the literal `i+48` will better express the nature of the conversion to the OP. – James Aug 24 '10 at 16:38
  • @Mark As long as you understand what it's doing, you can write it whatever way you want. – James Aug 24 '10 at 16:40
1

I don't believe that operator+=(int) is defined for std::string, so the line data += i; would, if it compiles at all, be translates as:

 data += (char) i;

Let's use actual chars here:

char i='0';   

while(i <= '5'){   
  i++;   
  data += i;   
  data += "\n";   
}

Furthermore, you are including <string.h> which is the C run-time library for strings (string here being arrays of tiny integers); What you actually need to include is <string> which is the C++ library for std::string. As far as I can tell, you do not need stdio.h, stdlib.h or string.h at all.

James Curran
  • 101,701
  • 37
  • 181
  • 258
1

Alternatively you can also use sprintf as:

    char temp[10]; // assuming your string rep of the number won't take >9 char.

    while(i <= MAX){
            i++;
            sprintf(temp,"%d",i);
            data += temp;
            data += "\n";
    }
codaddict
  • 445,704
  • 82
  • 492
  • 529