Why I am getting this output from this loop?

Question

#include <iostream>
using namespace std;

I am very new to cpp and programming and I am trying to find the factors of a number, max, why is my code output coming out the way it is?

int max;
cout << "Enter a number you'd like to see the divisors of: " << endl;
cin >> max;

//I am trying to find all divisors for the number max
//I know this isn't the most efficienct way but I thought that it would work.
//Instead of 50, 25, 20, 10, 5 ,1 for output it looks like 50, 25, 25, 25 25, 5 

for (int t=1; t <= max; t++) {
  if (max % t == 0) {
    int m = max/t; 
  }
} 
cout << m << endl;

you should not use the namespace https://stackoverflow.com/questions/1452721/why-is-using-namespace-std-considered-bad-practice — , Dec 26 '17 at 21:07
Even after adding a function around this code I'm pretty sure it won't compile (`m` is only declared inside the `if`-statement but it is accessed outside of this statement. Also, there is only one output with this code, i.e., it won't produce an error. To make this comment constructive, teaching something actually useful: *always* check if input was successful *after* trying to read, e.g., using if (std::cin >> max) { /* use max here */ }`. — Dietmar Kühl, Dec 26 '17 at 21:11
@BO41: `m` is initialized on definition. However, where it is printed it is actually out of scope. — Dietmar Kühl, Dec 26 '17 at 21:12

Ron · Answer 1 · 2017-12-26T21:45:42.507

3

Your output is misplaced. Move the cout << m << endl; statement into your if statement block:

if (max % t == 0) { // start of a block
    int m = max / t;
    std::cout << m << '\n';
} // end of a block

Make sure you properly mark the block of statements using braces {}. Now for a given input of 50 the output is:

50 25 10 5 2 1

Live example on Coliru

edited Dec 26 '17 at 21:45

answered Dec 26 '17 at 21:16

Ron

14,674
4
34
47

score 2 · Answer 2 · answered Dec 26 '17 at 21:41

using namespace std;

As BO41 said, you should never use the namespace, here are some reasons: Why is "using namespace std" considered bad practice?

Instead of using the namespace, you should write only what you are using, for example:

using std::cout;
using std::endl;

Now back to the question:

for(int t=1; t <= max; t++){
    if(max % t == 0)
        int m = max/t; 
} cout << m << endl;

Note that you are defining m inside the if and using it outside of it. also, if it wasn't for that, you would print only the last divisor you find. You should do something more like:

for(int t = 0; t <= max; t++){
    if(max % t == 0){
        int m = max/t
        cout << m << endl;
    }
}

here you will print every divisor of max. Personally, i would always open a block for if statements, even if there is only one line in the block, for me it's much more organized and may prevent errors.

score 0 · Answer 3 · answered Dec 27 '17 at 03:26

Is this your entire program? The variable

int m

is out of scope at the line

cout << m << endl;

which leads me to believe you have another variable named "m" declared earlier in the program that is being shadowed by the newly declared int also named "m" inside the if-block. If this is the case, then the previously declared variable "m" outside the if-block would get printed to cout.

Why I am getting this output from this loop?

3 Answers3