1

This is some code I wrote to check a string's presence in a file:

bool aviasm::in1(string s)
{
ifstream in("optab1.txt",ios::in);//opening the optab
//cout<<"entered in1 func"<<endl;
char c;
string x,y;
while((c=in.get())!=EOF)
{
    in.putback(c);
    in>>x;
    in>>y;
    if(x==s)
    return true;
}
return false;
}

it is sure that the string being searched lies in the first column of the optab1.txt and in total there are two columns in the optab1.txt for every row. Now the problem is that no matter what string is being passed as the parameter s to the function always returns false. Can you tell me why this happens?

Flexo
  • 87,323
  • 22
  • 191
  • 272
AvinashK
  • 3,309
  • 8
  • 43
  • 94
  • Why don't you add some traces to understand what your program is doing?! Like `cout << "x = \"" << x.c_str() '"' << '"' << x.c_str() "\", y = \"" << y.c_str() << "\", s = \"" << s.c_str() << '"' << endl;` inside the loop before `if(x==s)` would help. – Kashyap Sep 23 '11 at 16:17
  • @thekashyap...its funny i checked the code with ur suggestion and it returned true....my code works!! – AvinashK Sep 23 '11 at 16:44
  • well, I personally won't be happy if the code works but I donno why. My guess at the error is some issues with leading/trailing whitespaces. That's why I suggested you trace everything. – Kashyap Sep 23 '11 at 16:48

2 Answers2

5

What a hack! Why not use standard C++ string and file reading functions:

bool find_in_file(const std::string & needle)
{
  std::ifstream in("optab1.txt");
  std::string line;

  while (std::getline(in, line))  // remember this idiom!!
  {
    // if (line.substr(0, needle.length()) == needle)  // not so efficient
    if (line.length() >= needle.length() && std::equal(needle.begin(), needle.end(), line.begin())) // better
    // if (std::search(line.begin(), line.end(), needle.begin(), needle.end()) != line.end())  // for arbitrary position
    {
      return true;
    }
  }
  return false;
}

You can replace substr by more advanced string searching functions if the search string isn't required to be at the beginning of a line. The substr version is the most readable, but it makes a copy of the substring. The equal version compares the two strings in-place (but requires the additional size check). The search version finds the substring anywhere, not just at the beginning of the line (but at a price).

Kerrek SB
  • 464,522
  • 92
  • 875
  • 1,084
  • @avinash: For starters, you never reset the `x` string. Then the `y` string is seemingly unused. Finally, the `eof` check is broken. – Kerrek SB Sep 23 '11 at 16:34
  • @kerrek...what is meant by eof check is broken....and also how can i reset the x string – AvinashK Sep 23 '11 at 16:40
  • `in.get()` returns an `int` which is either a `char` value or EOF. Either your `char` type is signed and your code will terminate when reading a valid character (typically ÿ - small letter y with diaresis, U+00FF) or your `char` type is unsigned and it will never terminate. – Jonathan Leffler Sep 23 '11 at 16:54
  • @avinash: To reset `x` you can say `x = "";` or `x.clear()` at the beginning of a new line. For input, both `getline()` and formatted input are to be written inside the conditional as I do in my example; do *not* check for eof-ness manually. I added a more efficient search that doesn't make a copy, and another comparison for arbitrary finding, not just at the beginning of the line. – Kerrek SB Sep 23 '11 at 17:10
1

It's not too clear what you're trying to do, but the condition in the while will never be met if plain char is unsigned. (It usually isn't, so you might get away with it.) Also, you're not extracting the end of line in the loop, so you'll probably see it instead of EOF, and pass once too often in the loop. I'd write this more along the lines of:

bool
in1( std::string const& target )
{
    std::ifstream in( "optab1.txt" );
    if ( ! in.is_open() )
        //  Some sort of error handling, maybe an exception.
    std::string line;
    while ( std::getline( in, line )
            && ( line.size() < target.size() 
                 || ! std::equal( target.begin(), target.end(), line.begin() ) ) )
        ;
    return in;
}

Note the check that the open succeeded. One possible reason you're always returning false is that you're not successfully opening the file. (But we can't know unless you check the status after the open.)

James Kanze
  • 150,581
  • 18
  • 184
  • 329