0

So once I enter my first input, the error appears. I don't know what is the issue. Can anyone help?

vector<int> get_data(){
    vector<int> v;  //initialize the vector (vec)
    int temp;
    
    cin >> temp;  //get input (temp)
    
    while(isdigit(temp) == true){   //check input (while(isnumeric(temp))
        v.push_back(temp);  //if true store in vector
        cin >> temp;
    }
    return v;
}

/*void print (vector<int> vec){

    //for loop
    //get counter of the vec(length)
    //
}*/

int sum (vector<int> vec){
    float sum;
    for(int i = 0; i <= vec.capacity()-1; i++){
        sum += vec[i];
    }
    return sum;
}


int main(){
    vector<int> data_main;

    cout << "\nPlease enter your data." << endl;
    cout << "You can enter nondecimal words to notify the end of your data." << endl;
    
    data_main = get_data();

    //print(data_main);

    cout << "The sum of the vector elements is " << sum(data_main);

    return 0;
}

This is what happens:

Please enter your data.
You can enter nondecimal words to notify the end of your data.
10
zsh: segmentation fault
Tom Regner
  • 6,856
  • 4
  • 32
  • 47
  • 1
    Start by learning how to use a [*debugger*](https://stackoverflow.com/questions/25385173/what-is-a-debugger-and-how-can-it-help-me-diagnose-problems) to catch crashes as and when they happen. Then you can locate where in your code it happens, and also examine variables to make sure you don't have any null-pointers or go out of bounds of arrays or vectors. – Some programmer dude Apr 26 '23 at 23:45
  • As a possible hint: `for(int i = 0; i <= vec.capacity()-1; i++)` is ***wrong***. You can only loop up until the vector *size*, which is `vec.size()`. So change to `for(int i = 0; i < vec.capacity(); i++)`. Or use a [range-for loop](https://en.cppreference.com/w/cpp/language/range-for) like `for (int value : vec) { sum += value; }`. Or for your case [`std::accumulate`](https://en.cppreference.com/w/cpp/algorithm/accumulate) as in `sum = std::accumulate(begin(vec), end(vec), 0);` – Some programmer dude Apr 26 '23 at 23:48
  • Your current for loop (with `capacity()` instead of `size()`) will go *out of bounds* of the vector, leading to *undefined behavior*. – Some programmer dude Apr 26 '23 at 23:49
  • Also, `isdigit(temp)` is wrong as well. [`std::isdigit`](https://en.cppreference.com/w/cpp/string/byte/isdigit) checks if a *single character* is a single digit. Like e.g. `'1'`, `'6'` etc. The value in `temp` is already a number, with possibly multiple digits. – Some programmer dude Apr 26 '23 at 23:50
  • 1
    Also, `float sum;` doesn't initialize the variable `sum`, it's value will be *indeterminate*. Using indeterminate values in any way leads to *undefined behavior*. And why a floating point variable for integer only inputs? All in all, it seems you really need to invest in [some good beginners C++ books](https://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list) to read. And I recommend you start over from the very beginning of them. Whatever resource you're currently using isn't doing a very good job. – Some programmer dude Apr 26 '23 at 23:52
  • 2
    General rule of thumb: Whenever you see a `<=` in the exit condition of a loop, stop and take a closer look because you've probably found a bug. – user4581301 Apr 26 '23 at 23:53
  • 1
    Also please take some time to read [Why is "using namespace std;" considered bad practice?](https://stackoverflow.com/questions/1452721/why-is-using-namespace-std-considered-bad-practice) And while you don't show your includes (always show your includes, they might be relevant even if you don't know it), to be on the safe side please read [Why should I not #include ?](https://stackoverflow.com/questions/31816095/why-should-i-not-include-bits-stdc-h) – Some programmer dude Apr 26 '23 at 23:57
  • 2
    `vec.capacity()` does not do what you think it does. Look into `std::vector::size`. – Fantastic Mr Fox Apr 27 '23 at 00:26
  • `isdigit(temp)` looks weird. I suggest that, for debugging, you print out `temp`. From [the docs on `isdigit`](https://en.cppreference.com/w/cpp/string/byte/isdigit): _The behavior is undefined if the value of ch is not representable as unsigned char and is not equal to EOF_ . This means that if `temp` is larger than 255, you are in trouble. – user1934428 Apr 27 '23 at 06:41
  • 1
    I suggest that you remove the _zsh_ tag, as the question is not related to the Z-Shell. Actually, any shell would report that a program has crashed by a seg-fault. – user1934428 Apr 27 '23 at 06:44

1 Answers1

0

In your get_data function you have declared temp to be of type int and you read that in. Your while loop tests is that int is a digit with std::isdigit. This logic isn't correct because you're testing an int like it's a character, but it isn't.

You're then iterating based on the capacity rather than the size of the vector. This may access memory reserved for the vector but which has not been initialized in any meaningful way. This invokes undefined behavior.

You can avoid this issue entirely with a range-based for loop.

int sum(std::vector<int> vec){
    int s = 0;
    for (auto x : v) {
        s += x;
    }
    return s;
}

One more thing: you declared your sum function to return int but then declared your accumulator of type float.

Chris
  • 26,361
  • 5
  • 21
  • 42