CODEWITHSUNDEEP
c++loopsc++11vectorc++14
MKSnazzy
MKSnazzy

Reputation: 81

multiplying numbers in vector out of order in C++

My name is Matt. I'm new to StackOverflow and am fairly new to C++. Currently working my way through C++ Primer by Lippman.

I'm doing an exercise in the book, and task is to read integers in to the vector, and then multiply those integers by doing first and last, second and second to last, third and third last etc.

I did it myself without looking anything up, or else I'd barely learn if I just copied... my program compiles and acts as expected. My question is: did I do this correctly? is there a more efficient way of doing it?

I not only want to learn how to make working code, but I want to do it correctly. Thank you in advance!

#include <iostream>
#include <string>
#include <vector>
#include <cctype>

using std::cout; using std::cin; using std::vector; using std::endl;
using std::string;


int main()
{
    vector<int> numbers;
    int usernum = 0;

    cout << "Enter some numbers: ";
    while (cin >> usernum)
    {
        numbers.push_back(usernum);
    }

    unsigned maxElement = numbers.size() - 1;
    unsigned minElement = 0;

    for (auto i : numbers)
    {
        cout << numbers[minElement] << " * " << numbers[maxElement] << " = " << numbers[minElement] * numbers[maxElement] << "\n";
        ++minElement;
        --maxElement;
    }

    return 0;
}

Upvotes: 0

Views: 126

Answers (2)

andrgolubev
andrgolubev

Reputation: 885

  1. The first thing that feels strange for me is the namespaces you are using. Instead of doing: using namespace std::vector; you can fairly just do using namespace std; because you call std::vector anyways: vector<int> numbers;***. This applies to any "used" namespace you work with. Just do using namespace std; once and for all. ***I am unsure that std::vector/std::cout/... is even a namespace. std - is a namespace. std::vector should be a class under std namespace:

namespace std { template<typename T> class vector<T> {...}; }

  1. How come it "acts as expected". I do not get an idea of this loop: while(cin >> usernum). How do you know when the user input is finished from that point? For a first glimpse (did not compile/run it myself) I expect it either:

    • not compile
    • crash at runtime or having undefined behaviour
    • run the while-loop infinitely

  2. Use this instead: for (int i = 0, end_of_vector = numbers.size(); i < end_of_vector/2; i++) { cout << numbers[i] << " * " << numbers[end_of_vector - 1 - i] << " = " << numbers[i] * numbers[end_of_vector - 1 - i] << "\n"; } Reasons:

    • In this case you do not need any special variables to store first and last vector indexes.
    • You iterate only through a half of an array.
    • Using for (auto i : numbers), is is expected to use i as an element of numbers vector. But you do not do this, instead, you use numbers as is. Thus, this for-loop is ambiguous

Upvotes: 0

R Sahu
R Sahu

Reputation: 206687

In a comment, you said:

I've noticed it doesn't actually work as fully expected. It goes through vector and multiplies everything twice as maxElement goes right through to beginning, and minElement goes through to end. Not sure how to stop it once it's only done the operation on each one time.

If you don't want to repeat the multiplications, you need to change the for loop a bit.

for ( ; minElement <= maxElement; ++minElement, --maxElement)
{
    cout << numbers[minElement] << " * " << numbers[maxElement] << " = " << numbers[minElement] * numbers[maxElement] << "\n";
}

PS

When you use this logic, you'll need to make sure that minElement and maxElement are of a signed type. Otherwise, you will run into problems if numbers has only one element.

Upvotes: 1

Related Questions