Is my logic right?

[Satan]

void remove_coordinates(const unsigned int &index, coordinates *&path_array, unsigned int &current_size) {

    coordinates *new_path_array = new coordinates[current_size - 1];

    for (unsigned int i = 0; i < current_size; i++) {
        if (i == index - 1)
            continue;
        else if (i > index - 1)
            new_path_array[i - 1] = path_array[i];
        else
            new_path_array[i] = path_array[i];
    }

    current_size--;
    delete[] path_array;
    path_array = new_path_array;
}

void remove_coordinates(coordinates &coords_to_be_removed, coordinates *&path_array, unsigned int &current_size) {

    for (unsigned int i = 0; i < current_size; i++) {
        if (path_array[i].height == coords_to_be_removed.height && path_array[i].width == coords_to_be_removed.width)
            remove_coordinates(i, path_array, current_size);
        else
            cout << i << ' ';
    }
    cout << '\n' << current_size;
}

So I wrote a program which seems to be faulty, and my primary suspect is the above snippet of code. Faulty how? I don't get any output.. the kind of thing that happens when you're accessing an out of range index for example.


The first overload removes a 'coordinate' object from an array of coordinate objects by using the index, which, is FINE.

But I'm finding issue with the SECOND overload which takes a coordinate object instead of index as parameter and then removes the object from the array if it's found in the array.



I don't think it's necessary but here's the part of the code involved with calling the function anyway:

    while (covered_path_size > 0) {
        if (check_if_possible(current_position, p_p_maze, maze_height, maze_width)) {
            generate_next_path(current_position, p_p_maze, maze_height, maze_width);
            p_p_maze[current_position.height][current_position.width] = ' ';
            add_coordinates(current_position, covered_path, covered_path_size);
        }
        else {
            remove_coordinates(current_position, covered_path, covered_path_size);
            current_position = covered_path[0];
        }
    }

[MikeyBoy]

One thing springs immediately to mind:

In your second function (the "outer" one), you pass a zero-based index into the first (or "inner") one.

But in the inner function, you seem to treat that is if it were a one-based index; you subtract 1 from it before comparing it to the zero-based loop counter.

So, the entry that your inner function removes, is not the one that the outer function thinks it's telling it to remove.

EDIT: This sort of problem is exactly the sort of thing you should be using your debugger for. Step through the code, and see exactly what it's doing, for yourself.

[jonnin]

if I read that right, you have a for loop calling the top function which deletes the array inside it and then gets a new uninitialized pointer to replace it. second iteration of the for loop, what happens when you do THAT?? The [i] etc in the for loop look invalid after that to me!

I am not even 100% sure what you are trying to do (I can guess, but why 2 functions at all?), but it looks to be badly broken.

[Satan]

Ahh MikeyBoy that's very observant of you! I'm impressed!

And jonnin you're right I'm missing a break that would be included in the if statement, I forgot that too.
But note that it wouldn't be invalid because current_size is decremented when remove_coordinates is called so the for-loop does not access an out of index element. So it would instead remove all recurring elements (assuming that I fixed what MikeyBoy pointed out of course).

About using two functions, I wanted to write a function to remove coordinates from an array. I wanted one that would remove coordinates by index and another by searching for specified element. Any suggestions with regards to that?

[coder777]

I would suggest that you use vectors and other standard functions. Like:

http://www.cplusplus.com/reference/algorithm/move_backward/
http://www.cplusplus.com/reference/algorithm/find_if/?kw=find_if

[Satan]

Thanks coder777, I know STL functions are always preferred but I intentionally used pointers because I want to get used to using them, I have never used them before.

Other than the fact that STL functions are more safe, using vectors over arrays gives me no advantage in this program either way because I am only using arrays to represent the maze and a bunch of values, nothing else.. so it would make no difference using arrays or vectors to me in this case. What ya say?

[MikeyBoy]

The principle advantages of arrays include:
- memory management is all done for you
- various useful methods are already implemented; relevant to this particular program is the erase() method
- as STL containers, they can be used with various other standard library functions and algorithms; coder777 has already mentioned some that are relevant to this program.

Having said that, there's nothing at all wrong with doing this as a learning exercise for pointers and arrays.

[coder777]

I forgot to mention move(...). You can use std functions with arrays as well. The following example shows how to use move(...) and copy(...) with arrays:

void remove_coordinates(const unsigned int &index, coordinates *&path_array, unsigned int &current_size) {

    coordinates *new_path_array = new coordinates[current_size - 1];

std::move(path_array + index + 1, path_array + current_size, path_array + index); // Since you have a new array you may use another std::copy(...)
std::copy(path_array, path_array + current_size - 1, new_path_array);

    current_size--;
    delete[] path_array;
    path_array = new_path_array;
}

Not test.

[homy18]

Nwb, you declared 'index' and 'i' as 'unsigned int', but you used 'if (i == index - 1)'. I'm curious what will happen, if index = 0. Debugger should give a warning.

[Satan]

Good observation by you, homy. index - 1 would not return -1 if I had my indexes right.

Comparison between unsigned and signed is legal.

There is no problem with if(i == index-1) even if index is 0 and the two variables are unsigned int. Because here we are not assigning a negative value to either of the variables.. so the compiler should not complain.

(i == index - 1) does not change the value of any variable.. it's an 'expression'.
What the compiler see is this (when index is 0 and suppose i is 5):

if(5 == -1)
There's nothing wrong with this now is there?

The 'expression' would be illegal if it were comparing non-compatible data types like string and integer. But since signed can be represented as unsigned it's legal.

[homy18]

new_path_array[i - 1] = path_array[i];

i - 1 can be -1 ?

[jonnin]

I'm curious what will happen, if index = 0
------------------------------------------------
negative indexing generally works like you would think. if you have a byte pointer p[1000] to memory location 1000, and try p[-1] it will access memory at 999. This is useful now and then, consider
char * r = &(p[999]);
r[-100] = 10; //fine, this is just p[899].
Not sure if that should trip a warning or not, or if it would. I have not had a need to do this in years. It may not be standard, but its another thing most if not all compilers seem to support.

[Satan]

I guess VS 2017 doesn't support that index arithmetic jonnin, I got garbage value (and vector calls abort because of out of index call) when I tried index of [-1]. So I guess the pointer was accessing memory location before the array.. but it would be cool if [-1] was supported to access the second last element..

Mind if I ask which compiler you are using jonnin?

[jonnin]

Im using g++ these days but I am pretty sure vs will do it if you get a valid negative pointer.

try this.

char c[10];
char * cp = &(c[9]);
cp[-1] = 123;
cout << (int)(c[8]);


it won't work with vectors, most likely. they try to second guess you.
it might work with cp = &(vector[index]) though, with a negative on cp again. Vectors don't need this crap, they can reverse iterate. The only time I used this seriously was a bucket, so I could have buckets for negative values without having to offset each value. You may have to live with that not being viable with a vector.

[homy18]

Negative index can access neighbouring variables.

See this

#include <iostream>

using namespace std;

int main()
{
    cout << "Nagative Index Test:\n";
    
    char a[2] = {'Z','A'};
    char c[10] = {'1','2','3','4','5','6','7','8','9','0'};
	char d[2] = {'a','z'};
    
    char * cp = &(c[9]);
    cp[-1] = 123;
    
    cout << "\na = " << a[-1] << a[0] << a[1] << a[2];
    for(int i=-2; i<=11; i++)
        cout << "\nc[" << i << "]=" << c[i];//(int)(c[i]);
    cout << "\nd = " << d[-1] << d[0] << d[1] << d[2];
    return 0;
}

It's output is interesting.

Nagative Index Test:

a = 0ZA=
c[-2]=a
c[-1]=z
c[0]=1
c[1]=2
c[2]=3
c[3]=4
c[4]=5
c[5]=6
c[6]=7
c[7]=8
c[8]={
c[9]=0
c[10]=Z
c[11]=A
d = ◦az1

When you use index arithmetic, just beware of this and use better assert(index-i >=0) or assert(index+i < upper_bound).

[Satan]

The pointer is pointing to the 9th element so [-1] would be the 8th index. That's fine. But in my case new_path_array points to the beginning of an array so [-1] would be out of index.

Yes homy you're right that would be very wrong (sorry I forgot that there was an i-1 assignment) , your compiler might not warn you if you use static arrays but you are potentially corrupting data.

[jonnin]

Going off the positive end can do the same thing (tamper with other variables quietly, its actually better to crash it!).
its just memory locations. If you go to the wrong one from whatever path, you have problems!