Reflect Filter-less CS50

Question

Currently, I am doing pset4 filter-less, reflect and struggling with the code I wrote. It compiles fine, but the output picture looks exactly like the input picture. I am trying to first store the reflected image in a temporary array and transfer it to the image array. I was unable to find anyone who has tried something similar. This is what I have written so far.

void reflect(int height, int width, RGBTRIPLE image[height][width])
{
    int i;                              //row
    int j;                              //column for img
    int z;                              //column of tmp img
    RGBTRIPLE tmpi[height][width];      //tmp img
    for (i = 0; i < height; i++)
    {
        for (j = 0, z = width; j > z; j++, z--)
        {
            image[i][j].rgbtRed = tmpi[i][z].rgbtRed;
            image[i][j].rgbtBlue = tmpi[i][z].rgbtBlue;
            image[i][j].rgbtGreen = tmpi[i][z].rgbtGreen;
        }
    }
    for (i = 0; i < height; i ++)
    {
        for (j = 0; j < width; j++)
        {
            tmpi[i][j].rgbtRed = image[i][j].rgbtRed;
            tmpi[i][j].rgbtBlue = image[i][j].rgbtBlue;
            tmpi[i][j].rgbtGreen = image[i][j].rgbtGreen;
        }
    }
    return;
}

Can you please help me out?

Answer 1

You have 2 errors:

1. You first copy uninitialized memory.

    for (i = 0; i < height; i++)
    {
        for (j = 0, z = width; j > z; j++, z--)
        {
            image[i][j].rgbtRed = tmpi[i][z].rgbtRed;

The array tmpi contains indetermined values as you never assign anything to it.

You first need to copy the array to tmpi before you can copy it back.

2. Your limits for the loop are wrong:

        for (j = 0, z = width; j > z; j++, z--)
        {
            image[i][j].rgbtRed = tmpi[i][z].rgbtRed;
            ...

• Here, z may only go up to width-1.
• j>z is wrong condition. It will never be true.
• Proper condition would be j<width.
• Not an error but you don't really need z. Just use width-j-1
• Also not an error but there is no need to copy each member of a struct. You can just assign the whole struct at once.

A fixed version looks like this:

void reflect(int height, int width, RGBTRIPLE image[height][width])
{
    int i;                              //row
    int j;                              //column for img

    RGBTRIPLE tmpi[height][width];      //tmp img

    for (i = 0; i < height; i ++)
    {
        for (j = 0; j < width; j++)
        {
            tmpi[i][j] = image[i][j];
        }
    }


    for (i = 0; i < height; i++)
    {
        for (j = 0; j < width; j++)
        {
            image[i][j] = tmpi[i][width - j - 1];
        }
    }
    return;
}

Or you could even combine the outer loops:

void reflect(int height, int width, RGBTRIPLE image[height][width])
{
    int i;                              //row
    int j;                              //column for img

    RGBTRIPLE tmpi[height][width];      //tmp img

    for (i = 0; i < height; i ++)
    {
        for (j = 0; j < width; j++)
        {
            tmpi[i][j] = image[i][j];
        }

        for (j = 0; j < width; j++)
        {
            image[i][j] = tmpi[i][width - j - 1];
        }
    }
    return;
}

Or you could swap the values in place:

void reflect(int height, int width, RGBTRIPLE image[height][width])
{
    int i;                              //row
    int j;                              //column for img

    for (i = 0; i < height; i ++)
    {
        for (j = 0; j < width / 2; j++)
        {
            RGBTRIPLE temp = image[i][j];
            image[i][j] = image[i][width - j - 1];
            image[i][width - j - 1] = temp;
        }
    }
    return;
}

Feel free to think about more optimizations, once it is working correctly.