1

I am reading the lines in my .txt file. I got a problem in the last line. It wont read it. I have used eof() which is not a good practice. So I tried using the reading part as the parameters of my loop.

Here it is:

void browse(int accNum)
{
    int acnum,pin;
    float bal;
    string fname,lname;

    ifstream file;
    file.open("acc.txt",ios::in);

    if(file.is_open())
    {
        while(file>>acnum>>pin>>fname>>lname>>bal)
        {
            if(acnum==accNum)
            {
                cout<<"ACCOUNT NUMBER: "<<acnum<<"\nNAME: "<<fname<<" "<<lname<<"\nBALANCE: "<<bal;
                break;
            }
            else
            {
                cout<<"ERROR";
            }

        }
    }
    else
    {
        cout<<"FILE NOT OPEN";
    }
    file.close();
}

I've changed it to while((file >> acnum >> pin >> fname >> lname >> bal) != NULL). It is still not working. What is the solution for this? Here is the content of the database:

   1111111111 4324 James Doe 300000
    2222222222 0123 Eric Doe 10000
    1234567899 1234 John Doe 444444
guwop69
  • 119
  • 1
  • 13
  • I think you should show us the rest of your code in context. This looks very messy and may be making an easy problem much more difficult. – Tim Biegeleisen Oct 02 '15 at 05:21
  • If your text file has a newline at the eof, you can read it in like you already have. Delete `!=NULL` and the extra parenthesis. – kemotoe Oct 02 '15 at 05:26
  • Done. Seems to be on an infinite loop. – guwop69 Oct 02 '15 at 05:34
  • The code looks good to me. Please post the contents of "acc.txt". – R Sahu Oct 02 '15 at 05:42
  • I cant figure this out. – guwop69 Oct 02 '15 at 06:11
  • Are you sure you don't have code calling `browse` stuck in an infinite loop? Separately, the 2.22... billion account number is too big for a 32 bit signed integer - you could try a `long long` or a `std::string`. – Tony Delroy Oct 02 '15 at 06:36
  • 2
    Here's working code: http://coliru.stacked-crooked.com/a/41a12e1ba7266557 - note that I changed `fstream` to `istringstream` for testing purposes, and used `long long` to avoid an error parsing the 2-billion-odd account number value. (It prints ERROR twice because you print that for lines not matching the `accnum` argument.) – Tony Delroy Oct 02 '15 at 06:51
  • A while ago, it worked. But when I tried again it doesnt. – guwop69 Oct 02 '15 at 07:37
  • @TonyD Tried changing it to long long and copied everything. Still doesnt work. – guwop69 Oct 02 '15 at 07:38
  • Well, when you say it's stuck in an infinite loop, is it printing something while stuck (i.e. continuously writing output)? If not, add more `cout` statements - or use an interactive debugger - until you can see where the execution is not proceeding as you hope. You may want to redirect the output to `head` or `less` (if on UNIX/Linux) or `more` (Windoze) so you can see the initial part as it gets stuck. You can append that output to your question. If you like, also post the code calling `browse` so we can see any error in that. – Tony Delroy Oct 02 '15 at 07:41
  • That infinite is solved already. Tried debugging it again. It prints the two ERROR but still doest print the last line. – guwop69 Oct 02 '15 at 07:47
  • 1
    @guwop69 If your problem is not solved so far, please post a [MCVE] that we can reproduce your errors. – moooeeeep Oct 02 '15 at 07:48
  • @TonyD Your code works now! Before it didnt. It works fine now. thanks. – guwop69 Oct 02 '15 at 11:40

4 Answers4

3

Your file format (space separated values?) is broken. You can't discriminate the end of a name entry. std::cin will stop at the next whitespace and thus may tear the name apart. Reading the following balance will fail because it's actually still part of the name. There are names containing more whitespace than you'd expect, e.g., Guido van Rossum. You should use ';' or '\t' as a delimiter and use getline and stringstream for reading.

Also, you should not use float to represent money, because of the inherent inaccuracy of the representation. Consider this snippet: (Also have a look at the link I have put below for further reading.)

#include <iostream>
#include <iomanip>
int main() {
    float f = 123456789;
    // prints 123456792.000000
    std::cout << std::fixed << f << '\n';
}

Finally, as a commenter has observed, you need to make sure to use a large enough integer type to hold the account ID. Also consider using an unsigned integer type when you don't expect negative values, e.g., unsigned long long int or uint64_t. Note that life might be easier, when you would ignore the numerical properties of the account ID and just store it as a string instead.

For further reading:

Community
  • 1
  • 1
moooeeeep
  • 31,622
  • 22
  • 98
  • 187
  • Good point - the file format would be broken for real world use, where some people may not have exactly two words in their name, but for an assignment and given the first and last names are parsed separately, that's not actually causing trouble here. – Tony Delroy Oct 02 '15 at 07:04
0

It seems your file doesn't have an ending newline i.e. an extra line at the end of file. If you can alter the database and add an extra line the code will work.

If you cannot, you have to have a priming read for your while loop.

file >> acnum >> pin >> fname >> lname >> bal; before you start your loop and the same after your done performing all calculations inside the loop

kemotoe
  • 1,730
  • 13
  • 27
  • The code as posted already handled input with and without a terminating newline correctly. The remaining suggestions mirror harry's (though you were half an hour earlier), and my comments there apply. Cheers. – Tony Delroy Oct 02 '15 at 07:09
0

I think so you can use,

while(file)
{
   body;
 }

because as soon as you have read everything,there will be nothing in the file to read and so it will return NULL to the while loop and it stops.

harry
  • 45
  • 1
  • 10
  • Whether your suggestion works depends entirely on what's done before the `while` loop, and what's in the body... specifically, the code would need to attempt reading/parsing of one line/record beforehand, and attempt reading/parsing another at the end of the body. Still, it's redundant coding that twice, so this isn't the best approach (one of my comments under the question shows working code if you're curious). – Tony Delroy Oct 02 '15 at 07:07
  • Yeah,it is correct.Thank u very much, your reply is valuable to me. – harry Oct 02 '15 at 09:14
0

Here is one possible implementation:

// compare against a defined value 
int initilization_value = 10;
int acnum = initilization_value;  
// define  an input stream
ifstream file;
// attach it to the file with input mode (redundant as it is an input stream )
file.open("acc.txt", ios::in);
// check if file open
if(!file) cerr << "FILE NOT OPEN";
// extract input until the end of file
string line;
while(getline(file, line)){ 
    stringstream s(line);
    // in addition to that you can add a loop to differentiate the 
    // values extracted from s, depending on their type 
    // (isdigit(), isalpha(), isspace()), to populate your values 
    // in some special cases (as multiple names etc)
    s >> acnum >> pin >> fname >> lname >> bal;
    // test the input
    if(acnum != accNum){
        cout<<"wrong input variable: "<< anNum << '\n';
    }
    // all OK, exit the while loop  
    cout <<"ACCOUNT NUMBER: "<< acnum <<"\nNAME: "<< fname <<" "<<lname<<"\nBALANCE: "<< bal;
    break;
}
Ziezi
  • 6,375
  • 3
  • 39
  • 49
  • That's broken in all sorts of ways - a big step backwards from the code in the question. Specifically, it continues despite printing an message for `!file`, attempts input but doesn't check it worked before using it (a la `acnum == accNum` and the `cout` statement, won't handle empty input, and the loop termination condition would continue for a final line that's newline terminated despite there being no more usable input in the file. – Tony Delroy Oct 02 '15 at 06:54
  • @Tony D about : `if(!file) cout<<"FILE NOT OPEN";` you are simply wrong! Regarding the rest I will consider your remarks and edit it accordingly, thanks! – Ziezi Oct 02 '15 at 06:58
  • 1
    How am I "simply wrong"? Perhaps you misunderstand my point, which is that e.g. `if (!file) { cerr << "FILE NOT OPEN\n"; return; }` would be a better way to handle it. By not `return`ing, you fall into code that uses `acnum` uninitialised - that's undefined behaviour. – Tony Delroy Oct 02 '15 at 07:02
  • @Tony D I got it, thanks! In this case the same applies in the last line `cout << "ERROR"`. I just emphasized on the reading part and ignored the error messages competely:) – Ziezi Oct 02 '15 at 07:04
  • 1
    *"the same applies in the last line `cout << "ERROR"`"* - intuitively, but in fact that `"ERROR"` message is just reporting that the just-read/parsed line didn't contain the matching account number, and it needs to continue to the next line without breaking or returning. A message like `"still looking\n"` would have been infinitely better. – Tony Delroy Oct 02 '15 at 07:11
  • @Tony D you're right again and I though of changing it in read input till _the requirement is met_, rather than just notifying for wrong input, however I'm afraid if OP is overwhelmed by the amount information about the code – Ziezi Oct 02 '15 at 07:22
  • It doesnt work. But it prints the two ERROR and when it is in the last line it doest print. – guwop69 Oct 02 '15 at 07:43
  • @guwop69 check it now! :) – Ziezi Oct 02 '15 at 07:55
  • It seems to work for the 2nd and 3rd line. Now the problem is the 1st. It only prints 4 ERRORS – guwop69 Oct 02 '15 at 08:02
  • @guwop69 that is great! So, now we know that the _"reading last line"_ part of your question is covered. The ERRORS are possibly due to the `if (acnum == accNum)` , which are much more easier to solve, namely, you should extract only `acnum` and test it and THEN if it is true, extract the rest of the values. – Ziezi Oct 02 '15 at 08:10