0

I am a complete beginner to C++ and was assigned to write a function that returns the factors of a number. Below, I have included the function I also created called print_vector that will print all of the elements of a vector to the Console.

In my assignment, in order to check if the factorize function is working, we have to use the test_factorize function provided, which I have also included. However, the issue I've run in to is that the given test_factorize does not work due to the error "Initial value of reference to a non-const must be an lvalue." I'm unsure what this means and why the test_factorize runs into an issue because the output of factorize is a vector and the input of print_vector is also a vector, so I don't see why the contents of test_factorize result in an error, though I suspect it might be something within the `factorize' function I defined that causes this error.

#include <iostream>
#include <vector>

using namespace std;

void print_vector(std::vector<int>& v) {
    for (int i = 0; i < v.size(); i++) {
        cout << v[i] << " ";
    }
    cout << endl;
}

std::vector<int> factorize(int n) {
    std::vector<int> answer;
    for (int i = 1;i < n + 1; ++i) {
        if (n % i == 0) {
            answer.push_back(i);
        }
    }
    return answer;
}

void test_factorize() {
print_vector(factorize(2));
print_vector(factorize(72));
print_vector(factorize(196));
}
  • hi! This is one of the cases where `using namespace std;` is not a good idea; instead, `using std::cout;` would be much less confusing, potentially. (You'll have to use `cout << std::endl;` instead of just `cout << endl;`, that's the only change). `using` the whole `std::` namespace imports *a lot* of symbols. For example, there might be a `factorize` already in `std`! That's why people [will tell you](https://stackoverflow.com/questions/1452721/why-is-using-namespace-std-considered-bad-practice) not to do `using namespace std;`. – Marcus Müller Nov 23 '21 at 23:54
  • Also, you seem to be learning an old version of C++. Since '11, your `for(int i = 0; i < v.size(); i++) cout << v[i];` could be replaced with the much cleaner `for(int val : v) cout << val;`. – Marcus Müller Nov 23 '21 at 23:56
  • Thank you for the suggestion about namespace, I will definitely change that. As for the older version, I guess I am, I've only ever seen the old style syntax in my lectures. – Chloe Martinez Nov 24 '21 at 00:32

1 Answers1

2

The error is from this line:

void print_vector(std::vector<int>& v) {

Since you didn't include the const keyword in the argument-type, you are (implicitly) indicating that print_vector has the right to modify the contents of v.

However, you are calling print_vector() with a temporary object (the vector returned by factorize()) as an argument, and C++ doesn't allow you to pass a temporary object by non-const reference, presumably on the theory that making changes to a temporary object is pointless (because the temporary is going to be destroyed as soon as the function call returns, so any changes made to it would have no effect) and therefore must a programmer error.

In any case, the fix is easy, just change your function declaration to this:

void print_vector(const std::vector<int>& v) {

... and that will allow you to pass a reference-to-a-temporary-vector to it.

Jeremy Friesner
  • 70,199
  • 15
  • 131
  • 234