2
\$\begingroup\$

The algorithm should be checking if the block fits perfectly when it falls.

The block is always the same shape:

 .##....

   ..##....

Keep in mind I wrote this in 1 hour - all the time we had for the task. How can I make it more optimized?

#include <stdio.h>
#include <stdlib.h>
int check(char *k, int r, int s){
    int i, j, flag = 0;
    for (i = 0; i < r; i ++){
        for (j = 0; j < s; j ++){
            if (k[i * s + j] == '*' && k[(i + 1*2) * s + j + 1] == '.'){
                flag = 1;
            }
        }
    }
    return flag;
}
void toLadders(char *k, int r, int s){
    int i, j;
    for (i = 0; i < r; i ++){
        for (j = 0; j < s; j ++){
            if (k[i * s + j] == '*')
             k[i * s + j] = '#';
        }
    }

}
int main()
{
    int i, j, r, s;
    int flag = 0;

    do{
        scanf("%d %d", &r, &s);
    }while(r < 1 || s > 15);

    char matrix[r][s];

    for (i = 0; i < r; i ++){
        for(j = 0; j < s; j++){
            scanf(" %c", &matrix[i][j]);
        }
    }

    for (i = 0; i < r; i ++){
        for(j = 0; j < s; j++){
            if (matrix[i][j - 1] == '.' && matrix[i][j] == '#' && matrix[i][j + 2] == '.'
                && matrix[i + 1][j] == '.' && matrix[i + 1][j + 1] == '#' && matrix[i + 1][j + 2] == '#' && matrix[i + 1][j + 3] == '.'){

                matrix[i][j] = '*';
                matrix[i][j + 1] = '*';
                matrix[i + 1][j + 1] = '*';
                matrix[i + 1][j + 2] = '*';

            }
        }
    }

    for (i = 0; i < r; i ++){
        for (j = 0; j < s; j ++){
            if (matrix[i][j] == '*' && matrix[i + 1][j] == '.' && matrix[i + 2][j + 1] == '.' && matrix[i + 2][j + 2] == '.'){
                    matrix[i + 1][j] = '*';
                    matrix[i][j] = '.';
                    matrix[i + 1][j + 1] = '*';
                    matrix[i][j + 1] = '.';
                    matrix[i + 2][j + 1] = '*';
                    matrix[i + 1][j + 1] = '*';
                    matrix[i + 2][ j + 2] = '*';
                    matrix[i + 1][j + 2] = '.';
            }
        }
    }

    flag = check(&matrix[0][0], r, s);

    toLadders(&matrix[0][0], r, s);

    for (i = 0; i < r; i ++){
        for(j = 0; j < s; j++){
            printf("%c", matrix[i][j]);
        }
        printf("\n");
    }

    if (flag)
        printf("\nNot perfect!");
    else
        printf("\nPerfect!");

    return 0;
}
\$\endgroup\$
1
  • \$\begingroup\$ What is .##....? Is that supposed to be a picture of a Z shaped tetris piece? If so, the picture is not aligned. Plus, can you explain the problem in more detail? \$\endgroup\$
    – JS1
    Commented Dec 14, 2016 at 19:02

1 Answer 1

4
\$\begingroup\$

I honestly have no idea from your code what it is that it's supposed to do. I've run it several times and am still no better off.

Naming

Naming is an important aspect of coding. It makes your code much more approachable / self documenting. Almost none of your names give me any kind of hint as to what they represent or do. i,j,r,s They may mean something to you, but they don't to me. Put them together and you get this line if (k[i * s + j] == '*'). Give your variables meaningful names. Your method names are similarly abstract 'check', check what? 'toLadders' which seems to replace '*' with '#' for some reason.

Give your user a chance

You may write the best program in the world, but if you can't convince people to use it then it has almost no value. Your program needs some information from the user, however they aren't told what it is. You simply start scanning characters from stdin. Maybe this is ok because stdin is always redirected from a file, but if not tell your users what you're expecting. Ask them for input, then try to get it, don't just assume they're going to know what's expected of them.

Bounds Checking

Some of your bounds checking looks off to me. You create a matrix[r][s] then do this:

for (i = 0; i < r; i ++){
    for(j = 0; j < s; j++){
        if (matrix[i][j - 1] == '.' && matrix[i][j] == '#' && matrix[i][j + 2] == '.'
            && matrix[i + 1][j] == '.' && matrix[i + 1][j + 1] == '#' && matrix[i + 1][j + 2] == '#' && matrix[i + 1][j + 3] == '.'){

It looks a lot like when j=s-1, you're going to be accessing matrix[i + 1][s - 1 + 3], which is out of bounds...

Keep in mind I wrote this in 1 hour - all the time we had for the task.

Honestly, the amount of time you had initially to write the task is a side issue, you presumably had the option to revisit the code and make it the best you could offer before posting. Meaningful naming costs very little time and usually pays for itself with maintained understanding. It is the biggest step you could take to make your code more approachable. The more approachable your code is, the more value you are likely to get from reviews as reviewers will spend less time getting to grips with what you're doing allowing them to focus more on the how and why.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.