0

I am trying to to read each line from a text file and put in array column ,I really tried this :

    string buffer[256];
string a;
ifstream myfile ("1.txt");

for(i=0;i<10000;i++)
    {

        //readArrivalTimes(i);
        myfile.getline (buffer,100);
        a[i]=buffer;
    }

but It is not working

so I did try for one of the solutions you gave me guys and I did it like this :

std::vector<std::string> v;
std::string buffer;


string a[1024];
ifstream myfile;
myfile.open("1.txt");

for(i=0;i<n;i++)
{
    getline (myfile, buffer);

    a[i]= buffer;
    cout << buffer << "\n";
}

but as we can see it's string !

can we make it works as integer?

[Solved :)]

first of all thanks for everybody help me with this, I really appreciate your help, I am a totally new to c++.

and for sure it's not a homework.

I did some modifications to the code so it can works with integers

int a[1024];
ifstream myfile;
myfile.open("1.txt");

for(i=0;i<n;i++)
{
    getline (myfile, buffer);

    a[i]= atoi(buffer.c_str());
    cout << buffer << "\n";
}

thank you very much.

HAJJAJ
  • 3,304
  • 14
  • 38
  • 66
  • 1
    what does your debugger tell you? – pm100 May 21 '12 at 21:19
  • 1
    If you are using `string`s, then why not read the data into a `string` to begin with? Also, where is your _array of lines_? – K-ballo May 21 '12 at 21:19
  • this error is cming : 2 IntelliSense: a value of type "char *" cannot be assigned to an entity of type "int" c:\users\hajjaj\desktop\rr\roundrobin.cpp 57 8 RR – HAJJAJ May 21 '12 at 21:23
  • 2
    Do you want to read the entire file into one string, or into an array of strings, one string per line of the file? For the first see: http://stackoverflow.com/questions/3303527/how-to-pre-allocate-memory-for-a-stdstring-object/3304059#3304059. For the second, http://stackoverflow.com/questions/1567082/how-do-i-iterate-over-cin-line-by-line-in-c/1567703#1567703 – Jerry Coffin May 21 '12 at 21:23
  • now I had 4 errors : 1- 3 IntelliSense: no instance of overloaded function "std::basic_ifstream<_elem _traits="">::getline [with _Elem=char, _Traits=std::char_traits]" matches the argument list 2- 4 IntelliSense: a value of type "std::string *" cannot be assigned to an entity of type "char" 3- Error 1 error C2664: 'std::basic_istream<_elem> &std::basic_istream<_elem>::getline(_Elem *,std::streamsize)' : cannot convert parameter 1 from 'std::string [256]' to 'char *' 4- Error 2 error C2440: '=' : cannot convert from 'std::string [256]' to 'char' – HAJJAJ May 21 '12 at 21:31
  • 4
    Stop changing the code!! – Default May 21 '12 at 21:31

3 Answers3

2

There are many mistakes in your code.
Some are already mentioned, as assigning the char* to the int array.

But your approach is more C than C++. In C++ it would look more like this:

std::vector<std::string> lines;
std::ifstream myfile("1.txt");
if(myfile.isopen())
{
    std::string line;
    while(getline(myfile, line))
    {
        lines.push_back(line);
    }
}

I haven't tested it, but it should show you a way how to do this.

Edit: Changed the code according to comments.

Edit again to make it work with integers:

std::vector<int> numbers;
std::ifstream myfile("1.txt");
if(myfile.isopen())
{
    std::string line;
    while(getline(myfile, line))
    {
        int number;
        std::istringstream(line) >> number;
        numbers.push_back(number);
    }
}
Skalli
  • 2,597
  • 3
  • 25
  • 36
  • 4
    There are many mistakes in this code (an extra line is pushed onto lines). The while loop should look like: `while(getline(myfile, line))` – Martin York May 21 '12 at 22:08
  • 3
    No no no! `while (myfile.good())` is an iostream anti-pattern! You should use `while (getline(myfile, line))`. Downvoted for perpetuating that bad, bad style. – Jonathan Wakely May 21 '12 at 22:09
  • @JonathanWakely: Its not just iostream ant-pattern (unless you mean IO in the general language agnostic way). It is wrong in most languages. – Martin York May 21 '12 at 22:12
  • 1
    @LokiAstari, I meant C++ iostreams specifically. I won't comment on other languages but it annoys me that people insist on using iostreams like that when it makes the code longer and the results wrong. Doing it right (as in your comment) is not only less typing but actually works. Why do people do it the other way?! – Jonathan Wakely May 21 '12 at 22:19
  • @LokiAstari: Sorry, I didn't knew that. I've corrected my mistake and memorized it. – Skalli May 22 '12 at 08:06
  • @JonathanWakely: On about why people do it the other way: They don't know better. I learned it this way at an university, now I haven't worked much with textfiles in C++. I'm using C# at work and C++ on some hobby projects in my spare time. I'm glad you pointed out my mistake. I looked it up on [cplusplus.com](http://www.cplusplus.com/doc/tutorial/files/) and even there it is done in a similar way. *sigh* It's good to know better know, but how can we learn it better if we see it wrong all the time? I've corrected my mistake above to prevent from others doing the same mistake. – Skalli May 22 '12 at 09:05
  • cplusplus.com is a terrible reference, always has been. Those fstream examples all call `close()` which is redundant and usually unnecessary, let the destructor do it. – Jonathan Wakely May 22 '12 at 11:36
  • @JonathanWakely: I know that it is closed thanks to RAII at the end of the scope. I'll update my answer again and edit it out. Is there anything else redundant or wrong with my code? – Skalli May 22 '12 at 11:56
  • @Skalli: The way you did it is wrong in nearly all languages (including C#; they did not teach it this way at University (though your way is often taught as an anti-pattern)). The standard pattern is to do the read as part of the while test. – Martin York May 22 '12 at 15:08
  • Also there is no point in having `if(myfile.isopen())` unless you are specifically using it to report an error (and you are not). – Martin York May 22 '12 at 15:09
0

a is a string, which is an abstraction for a collection of characters.

a[i] will return a reference to an individual character.

buffer is an array of characters. The C/C++ convention in that the name of the array names a pointer to its first element.

So, what you are doing in your loop is assigning the ith element of a to the address of the beginning of your buffer, which is almost certainly not what you want to do.

Likely what you want to do is make a an array of strings; i.e. replace

string a;

with

string a[10000];

There's other things to address in your code; for example, what if your file has < 10k lines?

JohnMcG
  • 8,283
  • 5
  • 36
  • 49
  • He definately should go with `std::vector`! – Skalli May 21 '12 at 21:27
  • I'm guessing there's some homework restrictions here. But didn't want to ask. http://meta.stackexchange.com/questions/130558/can-we-cut-back-on-the-is-this-homework-berating – JohnMcG May 21 '12 at 21:29
0
    int a[1024];
ifstream myfile;
myfile.open("1.txt");
for(i=0;i<n;i++)
{
    getline (myfile, buffer);

    a[i]= atoi(buffer.c_str());
    cout << buffer << "\n";
}

this is the right answer

HAJJAJ
  • 3,304
  • 14
  • 38
  • 66
  • Hi, your code may work, but it's not good C++. It's pretty bad style. I've updated my code to work with integers too, have a look at it. – Skalli Mar 04 '13 at 17:34