Code Review Stack Exchange is a question and answer site for peer programmer code reviews. Join them; it only takes a minute:

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top

This question came from C++ Primer 5th edition:

Q: Read a set of integers into a vector. Print the sum of each pair of adjacent elements. Change your program so that it prints the sum of the first and last elements, followed by the sum of the second and second-tolast, and so on.

I was just wondering if there is any way to improve my solution.

Part 1:

int main(){

    vector<int> intVector;
    int number;
    while (cin >> number){
        intVector.push_back(number);
    }
    for (int i = 0; i < intVector.size(); i++){
        int k;
        k = intVector[i] + intVector[i + 1];
        cout << k << endl;
    }
    return 0;
}

Part 2:

int main(){

    vector<int> intVector;
    int number;
    while (cin >> number) {
        intVector.push_back(number);
    }
    int sizeOfIntVector = intVector.size();
    for(int i = 0; i < sizeOfIntVector / 2; i++){
        int k;
        k = intVector[i] + intVector[sizeOfIntVector - 1 - i];
        cout << k << endl;
    }

    return 0;
}
share|improve this question
up vote 2 down vote accepted

There are few things that I want to point out. First of all, I think that this part of code has an error

for (int i = 0; i < intVector.size(); i++){
    int k;
    k = intVector[i] + intVector[i + 1];
    cout << k << endl;
}

I think it will crash when i = intVector.size() - 1 and you are trying to access intVector[i + 1]. Try looping till i < intVector.size() - 1

Secondly, your second solution works fine when sizeOfIntVector is even and there is nothing to worry about. But when it is odd you are missing one sum to print. So I guess it should be something like this

int sizeOfIntVector = intVector.size();
int sumsToPrintCount = sizeOfIntVector / 2;
if (sizeOfIntVector % 2 == 1){
    sumsToPrintCount++;
}
for(int i = 0; i < sumsToPrintCount; i++){
    int k;
    k = intVector[i] + intVector[sizeOfIntVector - 1 - i];
    cout << k << endl;
}
share|improve this answer
  1. Never use using namespace std;. Some reasons why here

  2. Part 1 is fine other than the error Bogdan Yarema pointed out. I would personally just start at 1 instead thought.

    for (int i = 1; i < intVector.size(); i++){
    
        int k = intVector[i] + intVector[i - 1];
        cout << k << endl;
    }
    
  3. In case the size of the vector is odd and the element in center should be added to himself (as Bogdan Yarema pointed out as swell), it would be better to get the ceiling of intVector.size() / 2

    int half = (intVector.size() + 1 )/ 2;
    int last = intVector.size() - 1;
    for (int i = 0; i < half; i++) {
    
        int k = intVector[i] + intVector[last - i];
        std::cout << k << std::endl;
    }
    
share|improve this answer

For reading the input, I'd at least consider something along this line:

std::istream_iterator<int> input(std::cin), end;
std::vector<int> { input, end };

I'm not sure whether I really recommend it for this case, but the standard library has std::adjacent_difference. Despite "difference" in the name, it can actually carry out an arbitrary operation on adjacent items in a collection, so we could implement the first addition loop as:

std::adjacent_difference(in.begin(), in.end(), 
    std::ostream_iterator<int>(std::cout, "\n"),
    [](int a, int b) { return a + b; });

This does have one other oddity: it produces the first element of the input un-changed, then that's followed by the results for the pairs of elements.

For the second loop, we can use std::transform, (without the questionable details in the preceding):

auto mid = (in.size() + 1) / 2;

std::transform(in.begin(), std::next(in.begin(), mid),
    in.rbegin(),
    std::ostream_iterator<int>(std::cout, "\n"),
    std::plus<int>());

As I said, I'm uncertain whether I can really recommend using adjacent_difference to compute adjacent sums. Using transform instead of the second loop seems to me like a clear win though.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.