HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

Rotate an image (2D array) by 90 degrees recursively with low memory usage

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
imagelowarraywithdegreesrecursivelyusagememoryrotate

Problem

I'm looking for comments on coding style, passing arguments, ...etc.

```
#include

using namespace std;

void printImage(int** img, int N){

//check if image pointer is NULL
if(img == NULL){
return;
}

//check if any of the columns of the image are null (since its essentially an array of pointers)
for(int i=0; i return: do this by check if we need to go deeper into the "peels of the onion :)"
if(row > N-row-1 || col to store row or column being rotated
int* tmp = new int[N-row];

//backup row
for(int i=row; i=row ; i--){
img[i][col] = img[row][i];
}

//swap tmp with end row
for(int i=N-row-1 ; i>=row ; i-- ){
int tmpVal = img[col][i];
img[col][i] = tmp[N-i-1];
tmp[N-i-1] = tmpVal;
}

// Swap end row into first col
for(int i=N-(row)-2 ; i>=row ; i--){
int tmpVal = img[i][row];
img[i][row] = tmp[N-i-1];
tmp[N-i-1]=tmpVal;
}

// Swap first column backup into first row
for(int i=row+1; i<N-(row)-1 ; i++){
img[row][i] = tmp[i];
}

delete[] tmp;

do_rotateImageInplace(img, N, row+1, col-1);
}

void rotateImageInplace(int** img, int N){

// Check if image pointer is NULL
if(img == NULL){
return;
}

// Check if any of the columns of the image are null (since its essentially an array of pointers)
for(int i=0; i<N ; i++){
if(img[i] == NULL){
return;
}
}

// If image is less than 2 in size
if(N<2){
return;
}

do_rotateImageInplace(img, N, 0, N-1);
}

int main(){

//TEST CASES:
int N = 5;
int** img = new int*[N];
int val = 0;
for(int i=0; i<N ; i++){
img[i] = new int[N] {val++,val++,val++,val++,val++};
}

cout<<"* original image ****"<<endl;
printImage(img, N);

rotateImageInplace(img, N);

cout<<"* rotated image ****"<<endl;
printImage(img, N);

for(int i=0; i<N ; i++

Solution

First of all, I don't see why do_rotateImageInplace shall be recursive. And indeed, it is a tail recursive, and as such is trivially converted into a loop.

Second, the comments are seriously misleading. This is what they say:

//backup row
//copy row into col
    (hmm... now `col` is permanently destroyed)
//swap tmp with end row
    (hmm... the comment above says that tmp istelf is a row. Why would I want to swap rows?)


etc.

The only useful comment is about peeling the onion. And a good comment really wants to be a function name. So

for (int layer = 0; layer < N/2; layer++)
    rotate_layer_inplace(img, N, layer);


seems like what you are trying to do.

Code Snippets

//backup row
//copy row into col
    (hmm... now `col` is permanently destroyed)
//swap tmp with end row
    (hmm... the comment above says that tmp istelf is a row. Why would I want to swap rows?)
for (int layer = 0; layer < N/2; layer++)
    rotate_layer_inplace(img, N, layer);

Context

StackExchange Code Review Q#73680, answer score: 7

Revisions (0)

No revisions yet.