Creating Vectors in a For Loop

[fibbo]

Alright, I suspect there is already a solution here somewhere on the forums but I'm not quite sure I'm searching for the right terms so I hope you dont mind me asking the question anyway.

My goal is to make a constructor for a class called building.

instance variables are:

int _id; //id of the building
int _floors; // numbers of floors
int _rooms_per_floor; //rooms per floor
std::vector<std::vector<person*>> _assignments //this represents the building

now the constructor so far looks like this:

building::building(int id_, int floors_, int rooms_per_floor_)
  : _id(id_), _floors(floors_), _rooms_per_floor(rooms_per_floor_)
{

// TODO: creating the 2d vector _assignments with #_floors rows and #rooms_per_floor columns

}

The problem is how to create a 2d vector dynamically according to the number of floors and the number of rooms. If I just had a fixed number for both it would be obviously no problem, but I dont know how to approach this dynamically.

edit: if I had a fixed number for floors (lets say 3) and rooms (also 3) I'd proceed like this

std::vector<person*> floor_one;
floor_one.push_back(NULL);
floor_one.push_back(NULL);
floor_one.push_back(NULL);
std::vector<person*> floor_two;
//push 3 times with NULL
std::vector<person*> floor_three
//push 3 times with NULL

_assignments.push_back(floor_three);
_assignments.push_back(floor_two);
_assignments.push_back(floow_one)

thinking about it made me come up with this. could this work?

for (int i = 0; i<number_of_floors; ++i) {
_assignments.push_back(std::vector<person*>(_number_of_rooms);

}

[mike_2000_17]

>>thinking about it made me come up with this. could this work?

Yes it does. (well, you are missing one closing parenthesis)

But, there is a better way. The problem with your implementation is that you are doing a lot of useless copying (you create a "large" vector and push it to the _assignments vector). You should try to favor pre-allocation whenever possible, like so:

_assignments.resize(number_of_floors); //preallocate for the number of floors
for (int i = 0; i<number_of_floors; ++i) {
  _assignments[i].resize(_number_of_rooms); //resize each vector to the number of rooms.
  for (int j = 0; j<number_of_rooms; ++j)
     _assignments[i][j] = NULL;           //you should also set everything to NULL.
}

Also, this line:

std::vector<std::vector<person*>> _assignments //this represents the building

Should not compile for two reasons. First, you are missing the semi-colon at the end of the line. Second, when you have nested < > declarations, you need to leave a space between the > > signs, otherwise, the compiler interprets those as the operator >>

One final and important note: It is forbidden, in C/C++, to use a leading underscore in an identifier or any occurrence of a double underscore. These special names are reserved for the compiler for special purposes and cannot be used by the programmer. So, you need to change your habit and get rid of the _ at the start of the names. You can use either nothing at the start, or an "m" for "data member" (I often also use a leading "a" for parameters). But, whatever you choose, don't use a leading underscore (or a double underscore anywhere).

[fibbo]

I see! Thanks for that.

On the >> issue, it did compile (used xcode and g++ via terminal - I'm on a mac atm). Guess the compiler is smarter then me. Will try to avoid that in the future!

Now on that underscore note. You say its forbidden because its reserved for the compiler and I believe you but its just strange then that my compiler never said anything about it - or what am I missing? (also xcode and g++).
Anyway, this convention used in my code was given by the course I'm taking - will remember your method with a leading m or a then.

Should I go and tell the professor/teaching assistant about the leading underscore? :)

Thanks again for your help!

[fibbo]

I'm encountering now a problem with the deconstructor.

building::~building()
{
    for (estate::iterator it_floor = _assignments.begin(); it_floor != _assignments.end(); ++it_floor) {
		for (floor::iterator it_room = it_floor->begin(); it_room != it_floor->end(); ++it_room) {
			delete *it_room;
			*it_room = NULL;
		}
	}
}

//where floor and estate are typedef
typedef std::vector< std::vector<person*> > estate;
typedef std::vector<person*> floor;

Compiles fine and all but when I run the programm at the end I get a

residence_manager(4069) malloc: *** error for object 0x100100170: pointer being freed was not allocated
*** set a breakpoint in malloc_error_break to debug
Abort trap

That adress is a pointer to a still existing person (on whose member functions I can access right before I try to delete the pointer). What am I missing here?
And I also dont quite get what "set a breakpoint in malloc_error_break" means :)
fibbo

[mike_2000_17]

>>On the >> issue, it did compile (used xcode and g++ via terminal - I'm on a mac atm).

It is possible that your compiler accepts this >> use. It's just that many compilers don't accept it currently. The upcoming standard will fix that little problem and require that compilers compile the >> correctly in this use-case. But the current standard prescribes that this should not compile (but since it is more of a little annoying syntax glitch in the standard, many compiler makers choose to ignore that requirement).

>>its just strange then that my compiler never said anything about it - or what am I missing?

The leading underscore (and double underscore anywhere) is clearly stated in the standard as being reserved for compiler-specific things. The reason why the compiler will not complain is because the compiler compiles all the code, without distinction of what code is from a standard library it provides (like #include <something>) and user-code. The standard reserves the leading underscore for things that are used/provided by compiler-specific implementations in its standard libraries.

>>Should I go and tell the professor/teaching assistant about the leading underscore?

Yes. You can refer them to section 17.4.3.1.2/1 of the C++98 Standard, which states:

C++98 Standard section 17.4.3.1.2/1: (Global names)
Certain sets of names and function signatures are always reserved to the implementation:
— Each name that contains a double underscore (_ _) or begins with an underscore followed by an upper-case letter (2.11) is reserved to the implementation for any use.
— Each name that begins with an underscore is reserved to the implementation for use as a name in the global namespace.

>>Anyway, this convention used in my code was given by the course I'm taking

That's part of the difference between academic knowledge/teaching of C++ and practical experience.

>>What am I missing here?

I'm pretty sure that the problem is because you are deleting the pointer twice. Make sure that you don't happen to delete the same pointer twice. Make sure that all pointers are NULL if their point to nowhere. Make sure that you implement the Big Three.

>>That address is a pointer to a still existing person

Being able to access member functions or data is not an indication that the pointer was not deleted, don't rely on that as "evidence" that the pointer is still pointing somewhere valid.

>>And I also dont quite get what "set a breakpoint in malloc_error_break" means

Ignore that. A breakpoint is something that you can use in debug mode to stop the execution at a particular point and then step through the code line-by-line and evaluate variables in-between. This can be a very useful way to find mistakes. But, putting the breakpoint in malloc_error_break is meaningless (it's just that the compiler is too stupid to give you any better suggestions). To check for memory errors, you can use Valgrind. This program should be able to tell you exactly why this pointer deletion failed and give you a detailed trace of where the error originates from. But, I'm pretty sure that your error is with the Big Three.

[fibbo]

Will give the Big Three a shot. I guess I need to do that for both classes - building and person? I'd like to believe that it only matters for the person class really though. Since thats the only thing I ever copy/assign.
But cant hurt getting some training because on the first glance it doesnt look so easy!

Thank you again, Mike.

[mike_2000_17]

The Big Three is mostly important for any class that holds resources allocated from the heap (with new). So, it is most important for the "building" class. The person class, I would imagine, does not hold any dynamically allocated array via a pointer.

You should first try to make the building class non-copyable (by declaring the copy-constructor and assignment operator private and without an implementation).

[fibbo]

You're right, person doesnt hold any dynamically allocated variables.

I tried what you suggested (if I did it the right way)

I put this into my buildings.hpp file.

private:
   building operator= (const building& other){}
   building (const building& other){}

unfortunately the error remains.

Out of general curiosity: What can one read generally from such a backtrace. Just where the error occurred accessing which address?

(gdb) backtrace
#0  0x00007fff85465616 in __kill ()
#1  0x00007fff85505cca in abort ()
#2  0x00007fff8541d6f5 in free ()
#3  0x00007fff8833bbfc in std::basic_string<char, std::char_traits<char>, std::allocator<char> >::~basic_string ()
#4  0x0000000100002fc3 in person::~person (this=0x1001001a0) at person.hpp:13
#5  0x0000000100001c52 in building::~building (this=0x7fff5fbff5b0) at [...]/classes/building.cpp:27
#6  0x0000000100005e81 in main () at [...]/residence_manager.cpp:50

[mike_2000_17]

The non-copyable is implemented as follows:

private:
    building& operator= (const building& other); //notice & at return and no {}
    building (const building& other); //notice no {}

If the above triggers a compilation error, then it means that some part of your code (from where the error was triggered) does indeed do a copy of a building object, which is incorrect. I think you should post all the code for the building class. I'm sure the error will be obvious (to an experienced programmer).

[fibbo]

I found what the error caused and I thought since you invested time trying to help me I embarass myself by telling what I did wrong (and maybe there is even a way to make it better).

There is one function called (the bit that was wrong is at the very end of the function):

bool
building::swap_tenants(int first_tenant_, int second_tenant_) {
	
	bool found_t_one = false;
	bool found_t_two = false;
	int ft_one, ft_two, rt_one, rt_two;
	person* tmp;
	
	//finding indices for first tenant
	for (int i = 0; i<_floors; i++) {
		for (int j = 0; j<_rooms_per_floor; j++) {
			if (_assignments[i][j] != NULL) {
				if (_assignments[i][j]->get_id() == first_tenant_) {
					ft_one = i;
					rt_one = j;
					found_t_one = true;
					break;
				}
			}
		}
	}
	
	//finding indices for second tenant
	for (int i = 0; i<_floors; i++) {
		for (int j = 0; j<_rooms_per_floor; j++) {
			if (_assignments[i][j] != NULL) {
				if (_assignments[i][j]->get_id() == second_tenant_) {
					ft_two = i;
					rt_two = j;
					found_t_two = true;
					break;
				}
			}
		}			
	}
	
	if (found_t_one && found_t_two) {
		tmp = _assignments[ft_one][rt_one];
		_assignments[ft_one][rt_one] = _assignments[ft_two][rt_two];
		_assignments[ft_two][rt_two] = tmp;
                // HERE WAS THE MISTAKE: delete tmp;
		return true;
	}
	else
		return false;

}

As you see I do a tmp pointer and delete it before return. Now when calling the deconstructor, I tried to delete that pointer again, which caused the error.

But looking at the code myself, it seems like a sh*t load of code for nothing more but switching tenants. Of course I've to find the i,j to know in which room the tenant is but still, is there a more elegant way to solve this? Apart from giving person member variables that save the floor and room.

Also yesterday I tried to implement my own assignment operator for person (just to get some experience, but I guess the hard part comes when member variables are pointers) but this should do the right stuff?

person& 
person::operator= (const person& that) {
	if (this != &that) {
		_name = that._name;
		_surname = that._surname;
		_address = that._address;
		_id = that._id;
	}	
	return *this;
	
}

Overall, the task is solved, compiles without errors and I'm happy so far. Thanks to mike :)

[mike_2000_17]

>>it seems like a sh*t load of code for nothing more but switching tenants.

Well, you could merge the two loops and merge the nested if-statements. Also, note that (_assignments[i][j] != NULL) is equivalent to (_assignments[i][j]) in an if-statement. So, you get:

//finding indices for first tenant
  for (int i = 0; i<_floors; i++) {
    for (int j = 0; j<_rooms_per_floor; j++) {
      if (_assignments[i][j]) {
        if (_assignments[i][j]->get_id() == first_tenant_) {
          ft_one = i; rt_one = j;
          found_t_one = true;
        } else if (_assignments[i][j]->get_id() == second_tenant_) { 
          ft_two = i; rt_two = j;
          found_t_two = true;
        };
        if( found_t_one && found_t_two )
          break;
      }
    }
  }