Enum, array, void functions

[Naulee]

So I basically have no idea what I'm doing as my instructor did not give very useful information on how to utilize enumerators.
The purpose of the program is to use three functions to first initialize all 150 mailboxes to close. The second function "beginning with mailbox 2, open the doors of all the even-numbered mailboxes, leaving the others closed.
Next, beginning with mailbox 3, every third mail box, opening its door if it were closed, and closing it if it were open. Then repeat this procedure with every fourth mailbox, then every fifth mailbox, and so on."
The third function will print which of the 150 mailbox doors are closed. Does my code so far look even remotely usable? I've been able to build it but haven't been able to get it to actually print. I know my functions are probably not right...

#include <iostream>
using namespace std;
 
enum mailbox{open, close};
const int NUM_BOX = 150;
int box[NUM_BOX];
 
void closeBox()
{
	int i;
	for (i=0;i<NUM_BOX;i++)
	{
		box[i] = close;
	}
}
 
void openBox()
{
	int e;
	for (e=2;e<=NUM_BOX;(e+2))
	{
		box[e] = open;
	}
 
	int t = 3;
	int k = t;  
 
	for (t;t<=NUM_BOX;t++)
	{
		for (k;k<=NUM_BOX;(k+k))
		{
		if(box[k]==close)
		{
			box[k] = open;
		}
		else
		{
			box[k] = close;
		}
		}
	}
}

void closedBox()
{
	int j;
	for (j=1;j<=NUM_BOX;j++)
	{
		if (box[j] = close)
		{
		cout<<"Door "<<j<<" is closed"<<endl;
		}
	}
}


int main()
{
	closeBox();
	openBox();
	closedBox();

	return 0;
}

[LB]

- You should try to avoid using global variables, use parameter/argument passing instead
- "closedBox" is an ambiguous and potentially confusing name, it should probably be "countClosedBoxes"
- You could use booleans instead of a weakly typed enum here
- Your for loop on line 20 is malformed; it is valid syntax but it doesn't do anything close to what you wanted it to. Same for line 30. Hint: + is not the same as +=

By the way, one of my pet peeves is people using the phrase "void function". Functions are functions no matter what their return type is, and you shouldn't refer to them by their return type (especially since their return type is of almost no importance with overloading functions).

[AbstractionAnon]

In addition to what LB pointed out, you also have invalid array references due to faulty for loops at lines 20, 28, 30 and 47.

C++ arrays are zero based. Your valid boxes are [0]-[149]. Your loops will cause a reference to box[150], which is not valid.

Lines 36-39 are unneeded. If the box is closed, there is no reason to set it closed.

Line 49, this is an assignment statement, not a comparison. = is the assignment operator. == is the comparison operator. This statement will cause all boxes to be set to closed and the if test to return true (close is 1).

@LB - closeBox and closedBox are confusing, but not ambiguous. I misread them the first time too and agree with you that countClosedBoxes would be a better function name.

[LB]

I meant the name was ambiguous from the point of view of "I just saw the name and need to guess what it does", not the signature of the function.

[Naulee]

Okay so I know my valid boxes are 0 to 149, but I need them offset by 1 so that the cout will print 1 to 150.
The purpose for lines 35 to 38 is supposed to be when the program loops back through, it should find open doors and close them and find closed doors and open them. The problem I'm having now is getting the loops to actually go back 150 times, starting at the next number in the sequence each time.
(So there is something wrong with my for loops at 27?)

#include <iostream>
using namespace std;
 
enum mailbox{open, close};
const int NUM_BOX = 150;
int box[NUM_BOX];
 

void closeBox()
{
	int i;
	for (i=1;i<=NUM_BOX;i++)
	{
		box[i] = close;
	}
}
 
void openBox()
{
	int e;
	for (e=2;e<=NUM_BOX;(e+=2))
	{
		box[e] = open;
	}
	
	
	for (int k=3;k<=NUM_BOX;k++)
	{
		for (k;k<=NUM_BOX;(k+=k))
		{
			if(box[k]==close)
			{
			box[k] = open;
			}
			else if(box[k]==open)
			{
			box[k] = close;
			}
		}
	}
}
void countclosedBox()
{
	int j;
	for (j=1;j<=NUM_BOX;j++)
	{
		if (box[j] == close)
		{
		cout<<"Door "<<j<<" is closed"<<endl;
		}
	}
}


int main()
{
	closeBox();
	openBox();
	countclosedBox();

	return 0;
}

[AbstractionAnon]

Okay so I know my valid boxes are 0 to 149, but I need them offset by 1 so that the cout will print 1 to 150.

So just add 1 when you print the box number.

    for (j=0; j<NUM_BOX; j++)  // initialization and terminal condition fixed
    {   if (box[j] == close)
            cout << "Door "<< j+1 << " is closed" << endl;
    }

Lines 12, 21, 27, 29, 45: These for loops will still generate illegal references to box[150].

The purpose for lines 35 to 38 is supposed to be when the program loops back through, it should find open doors and close them and find closed doors and open them.

You're missing the point. The else if on line 35 is not needed. If a box is not closed, then it must be open.

        if (box[k]==close)
             box[k] = open;
        else     // box must be open 
            box[k] = close;

Lines 27, 29: You're using the same loop variable (k) for both of these loops. IMO, this is a poor practice. Frankly, I can't tell what you're trying to do with these two loops and what is supposed to happen to k. I recommend using a different loop variable for the inner loop. I think this will fix your loop problem. In any case, it will make your debugging easier.

    for (int q=k; q<NUM_BOX; q+=k)
    {   if (box[q]==close)
             box[q] = open;
        else     // box must be open 
            box[q] = close;
    }

[Naulee]

Okay I followed what you said exactly, and it worked, thank you very much!