-3

I am new to the C++ language and trying to complete a very simple code challenge to sum up all the numbers in an array. The test environment doesn't show an error message it only gives me an exit code 139. Upon some research this means my code is producing undefined behavior (also maybe memory fragmentation?). Is it just something with the syntax or is there something I'm missing about C++?

#include <vector>

int sum(std::vector<int> nums) {
  int runningSum = 0;
  for (int i=0; i <= nums.size(); i++) {
    runningSum = runningSum + nums[i];
    }
  std::cout << "The total sum for nums: " << runningSum;
}
ejbee3
  • 62
  • 5
  • 7
    think about which element of the vector your loop tries to access in its last iteration… – Michael Kenzel Nov 19 '19 at 16:37
  • 2
    The standard library provides `std::accumulate` for this. Your problem is `<=` should be `<`. – Mansoor Nov 19 '19 at 16:37
  • 1
    VTC as a typo. The valid indices into a container should be the first thing you learn. If you haven't learned that yet, use `at()` instead of `operator[]` until you have. – underscore_d Nov 19 '19 at 16:38
  • 1
    Take a minute to look at the differences between your loop and [others' loops](https://stackoverflow.com/q/12702561/2602718). (Note: this should've been the first thing you did before posting here. The downvotes you're receiving are likely due to a lack of research) – scohe001 Nov 19 '19 at 16:39
  • 4
    That function looks like it's supposed to return the result, not print it. – molbdnilo Nov 19 '19 at 16:40
  • Right. It has UB because it returns no value. Another basic typo/thinko. – underscore_d Nov 19 '19 at 16:41
  • Suggestion: compile your program with "g++ -g -o PROGRAMNAME FILENAME.cc" and start the program in gdb, which is a debugger. A tutorial might help: http://www.gdbtutorial.com/tutorial/how-use-gdb-example – Semo Nov 19 '19 at 16:52

3 Answers3

3

Your function is supposed to return. You don’t, so you have undefined behavior there.

Your loop goes one beyond the last element of the vector, that has another UB.

Here is the fixed version:

int sum(std::vector<int> nums) {
  int runningSum = 0;
  for (int i=0; i < nums.size(); i++) {
  //            ^^^^
    runningSum = runningSum + nums[i];
    }
  std::cout << "The total sum for nums: " << runningSum;
return runningSum;
//     ^^^^
}

Please note that std::accumulate can do the same for you.

And as @gast128 suggested, your function signature can be changed to avoid copying the vector:

int sum(const std::vector<int>& nums)
Oblivion
  • 7,176
  • 2
  • 14
  • 33
0

Sorry ya'll yes I just had to return the result. I also had to remove the <= and replace with < but leaving <= gave me an exit code 139.

ejbee3
  • 62
  • 5
  • 1
    ...which is what Oblivion said, so it seems best just to accept their answer and add this as a comment, rather than adding your own copy of the answers others already gave. – underscore_d Nov 19 '19 at 16:50
0

I'd also like to add a quick note on the use of std::vector<>::operator[].

From the documentation here, you can read that it returns the reference of the element in the vector. But unlike std::vector<>::at, using the operator[] with a number n >= vector.size() causes undefined behavior.

If you had used std::vector<>::at instead, it would have thrown an out_of_range exception, which is more talkative.

Andreas Wenzel
  • 22,760
  • 4
  • 24
  • 39
lucie
  • 895
  • 5
  • 19