0

I'm not getting any errors, just an infinite loop! Here's my code:

#include<iostream>
#include<string>
#include<vector>
#include<algorithm>
#include<cmath>
#include<fstream>
#include<assert.h>
using namespace std;

inline void keep_window_open() { char ch; cin >> ch; }

int main() {
    int sum{ 0 }, number;
    vector<int> numbers;
    string word;
    fstream file;
    file.open("file1.txt", fstream::in);

    while (true) {
        file >> number;
        if (file.eof()) {
            numbers.push_back(number);
            break;
        }
        else if (file.fail()) { file >> word; }
        else if (file.bad()) exit(1);
        else numbers.push_back(number);
    }

    for (int x : numbers) sum += x;

    cout << sum << "\n";
}

The file I am reading from:

words 32 jd: 5 alkj 3 12 fjkl 
23 / 32
hey 332 2 jk 23 k 23 ksdl 3
32 kjd 3 klj 332 c 32
Remy Lebeau
  • 555,201
  • 31
  • 458
  • 770
Cluelesshint
  • 33
  • 5

2 Answers2

1

You are not reading the integers properly, or correctly checking if operator>> is successful before using number. And more importantly, you are not clearing the stream's error state when non-integer input is encountered by operator>>.

Try something more like this instead:

#include <iostream>
#include <string>
#include <vector>
#include <algorithm>
#include <fstream>
#include <stdexcept>
using namespace std;

inline void keep_window_open() { char ch; cin >> ch; }

int main() {
    int sum = 0, number;
    vector<int> numbers;
    string word;
    ifstream file("file1.txt");

    while (file >> word) {
        try {
            number = stoi(word);
        }
        catch (const std::exception &) {
            continue;
        }
        numbers.push_back(number);
    }

    if (file.fail() && !file.eof())
        exit(1);

    for (int x : numbers)
        sum += x;

    cout << sum << "\n";
    keep_window_open();

    return 0;
}

Live Demo

Remy Lebeau
  • 555,201
  • 31
  • 458
  • 770
  • I just cleared the fail state after if (file.fail()) { file.clear(); file >> word;} //this worked – Cluelesshint Aug 24 '20 at 19:27
  • When we lock all commenting on a post because people need to calm down a bit the idea is that you *walk away and calm down a bit*. Moving the heated exchange to another comment section is missing the point, namely that comments are not for extended discussion. – Martijn Pieters Aug 25 '20 at 00:34
-6

See if this works:

#include<iostream>
#include<fstream>
#include <cctype>    // isdigit
#include<string>
#include<vector>

using namespace std;

inline void keep_window_open() { char ch; cin >> ch; }

int main() {
    int sum{ 0 }; 
    string number;    // new
    vector<int> numbers;
    fstream file;
    file.open("file1.txt", fstream::in);

    while (true) {
        file >> number;
        if (file.eof())
            break;
        if (isdigit(number[0]))     // new
            numbers.push_back(stoi(number));
    }
    file.close();

    for (int x : numbers) sum += x;

    cout << sum << "\n";

return 0;
}
Giogre
  • 1,444
  • 7
  • 19
  • This is going to be an issue if a string starts with a digit, but has non-digits after it. – cigien Aug 24 '20 at 19:18
  • `while (true) { file >> number; if (file.eof()) break; ... } ` should be `while (file >> number) { ... }` Especially since `operator>>` can return a string value AND set `eofbit` at the same time when it reaches the last character in the stream. – Remy Lebeau Aug 24 '20 at 19:25
  • @RemyLebeau: `number` is a `string`. I left the infinite loop in place because I prefer to adhere to the original version of the code as much as possible. It's no use to OP if answers are too far away from original, and they are just so to prevent corner cases. – Giogre Aug 24 '20 at 19:29
  • @cigien: I checked for mixed alphanumeric words in the input provided by OP, found none, so code is appropriate for the job. – Giogre Aug 24 '20 at 19:35
  • 1
    @Lingo "*I left the infinite loop in place because I prefer to adhere to the original version of the code as much as possible*" - the original code was broken to begin with. Using `>>` directly in the `while` is the correct and *preferred* way to write such a reading loop. – Remy Lebeau Aug 24 '20 at 19:41
  • 1
    Bad practice to check the read separately. Prefer to use the idiomatic way to read a file `while (file >> numbre)` – Martin York Aug 24 '20 at 19:42
  • 1
    Manually calling close on a file is bad practice. In the general case this can cause an exception. Let the destructor do that for you. – Martin York Aug 24 '20 at 19:44
  • 1
    The `return 0;` at the end is non idiomatic. Its not need in C++ and implies that there is an alternative exit path that can return non zero somewhere else in the code. No such exit exits so you are confusing the reader. – Martin York Aug 24 '20 at 19:46
  • I'm not sure what you mean by the code is appropriate for the job, because it passes the single test case. Would `int main() { cout << 889; }` be an acceptable solution then? I'm afraid this answer is not useful, as it doesn't really answer the question. – cigien Aug 24 '20 at 20:18
  • @Martin York, et al: this answer fixes OP's problem while making as few modifications to his original code as possible. The code in the answer can be immediately understandable by OP. Hence your criticisms should have been placed beneath the original question, not here. – Giogre Sep 12 '20 at 17:52
  • Regarding the `return 0;` statement, my IDE, Code::Blocks, adds it to every main file in a new project. All other disapproving comments border on the ludicrous: *Manually calling close on a file is bad practice*... Well, the `close()` command is still not deprecated, isn`t it? Moreover, as far as I know any resource acquired must be released: better keeping to the principle instead of creating asymmetric code that may result confusing to the reader, just in order of taking care of corner cases. – Giogre Sep 12 '20 at 17:53
  • @Lingo: If you don't want to learn that does not bother us, just ignore the comments. But these comments are not just for you. We are also targeting other engineers looking for good advice on best practices to make sure your advice is documented where it deviates from best practices. Your opinions on are advice show a C style mentality to coding that was left behind by the C++ community over 10 years ago. Time to update your C++ skills to modern practices or stick to advice on C :-) – Martin York Sep 13 '20 at 18:15
  • @Lingo: PS: `file >> number;` is still fundamentally broken code even with your new addition and even if you ignore the other advice. – Martin York Sep 14 '20 at 21:27
  • @Martin York: `while (true) { file >> number;` is taken verbatim from OP code. Again, I just took his code and corrected some lines to achieve the task he wanted with it. You should put your comment directly under his question. I am not here to proselytise on those mythical best practices you seem so fervent about. I have already been pilloried enough as a deplorable heathen still adherent to the old C ways in this thread, enough is enough. Comment under the OP question, or write down your own answer. – Giogre Sep 14 '20 at 21:47
  • @Lingo Yes I can see it is taken verbatim; but because it is broken your fix does not work. – Martin York Sep 14 '20 at 22:15
  • @Martin York: works perfectly, my `number` is a string – Giogre Sep 14 '20 at 22:17
  • If the `operator>>` fails. Then your code is still executed. But the content of `number` has not changed and thus you code is working on the content of the previous read that worked not the current read. Thus if the last item is a number it will be pushed twice. To fix this you need to move your code from its current position to after the test for eof (even though that line is not perfect). – Martin York Sep 14 '20 at 22:20
  • of course, but the OP's input file is safe in this regard. I believe he was less concerned about exception safety than running what he needed. Are you sure he wanted the code of his 20 lines script be turned upside down, because of the theoretical existence of some obscure corner case he would never ever stumble on anyway? How can he repent now for violating the scriptures? Should he recite which item from Meyers' book loudly three times? Or is alas Meyers a pagan too? His book is way too old.. – Giogre Sep 14 '20 at 22:30