Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

The puzzle is:

  1. The deck contains 21 unique numbers
  2. The numbers should be laid up in rows of 7 (3 rows in total)
  3. ask the player to memorize a number to start the game
  4. once the game starts, rearrange the numbers in random in all rows.
  5. ask the player to select the row the number is in.
  6. once the player selects, rearrange the numbers for a second time
    1. this time, it is not random but sort the numbers in bundles per row
    2. stack all the bundles on top of each other, keeping the row selected to be in the centre.
    3. rearrange it back into rows of 7 such that first number in stack goes to row 1, second to row 2, third to row 3, 4th to row 1 again and so on.
  7. again ask the player to select the row the number is in.
  8. repeat step 6
  9. again ask the player to select the row the number is in.
  10. point to the middle number of the row selected as the answer.

You can use any programming language to solve this.

I'd like feedback on this.

#include "stdafx.h"
#include <stdio.h>
#include <stdlib.h>
#include <time.h>
#define numRows 3
#define numCols 7
#define TotalNum (numRows*numCols)

void random_rearrange_num(int p[][numCols]) ;
void display(int p[][numCols]) ;
void start();
int row_select(int count);
void stackup(int p[][numCols],int *s,int count);
void back_to_array(int p[][numCols],int *s);


int main()
{  
    //for random numbers
    srand(time(NULL)); 

    //Step 1
    int arr[numRows][numCols] = {{0,1,2,3,4,5,6},{7,8,9,10,11,12,13},{14,15,16,17,18,19,20}};
    int stack[TotalNum] = {0};
    int row = 0;

    //Step 2
    display(arr);

    //Step 3
    start();    

    //step 4
    random_rearrange_num(arr);

    //Step 5 (first time)
    display(arr);
    row = row_select(row);

    //Step 6
    stackup(arr,stack,row);
    back_to_array(arr,stack);

    //Step 7 (second time)
    display(arr);
    row = row_select(row);

    //Step 8:repeat step 6
    stackup(arr,stack,row);
    back_to_array(arr,stack);

    //Step 9 (third time)
    display(arr);
    row = row_select(row);

    //Step 10 and Step 11
    printf("The answer is : %i",arr[row][3]); 

    return 0;
}
void random_rearrange_num(int p[][numCols])
{
  int temp = 0,k= 0,l = 0;
  for(int i = numRows-1 ; i > 0 ; i--)
  {
    for (int j = numCols-1;j>0; j--)
    {
          k = 0 + rand()/(RAND_MAX/(2-0+1)+1);
          l= 0 + rand()/(RAND_MAX/(6-0+1)+1);
          temp = p[i][j]; 
          p[i][j] = p[k][l];
          p[k][l] = temp;
    }
  }
}

void display(int p[][numCols])
{
       //printf("\n");
       for (int i = 0; i< numRows;i++)
       {
          for ( int j = 0;j< numCols;j++)
          {
            printf("%8i",p[i][j]);
          }
          printf("\n");
       }
}

void start()
{
        printf("\n");
        printf("Please memorize a number(between 0 to 20) and press enter to continue");
        char c=getchar();
        while (c != '\n')
            c=getchar(); 
        printf("\n"); 
}
int row_select(int count)
{
     printf("\n");
     printf("Please select the row the number is in(between 1 to 3):");
     scanf_s("%i",&count);
     return count-1;
}


void stackup(int p[][numCols],int *s,int count)
 {         
    int k = 0,t= 0;  
    int middle = (numRows-1)/2;  
    for (int i = 0; i<numRows; i++)
    {        
        t=i;      
        if (i==middle || i==count)  
            i = (i==count)?middle:count;  
        for (int j = 0; j<numCols; j++)    
        {
            s[k++] = p[i][j];
        }
        i=t;    
    } 
} 
void back_to_array(int p[][numCols],int *s)
{
  int k = 0;
  for (int i = 0; i< numCols;i++)
       {
          for ( int j = 0;j<numRows;j++)
          {
             p[j][i] = s[k++];
          }

       }
}
share|improve this question
    
This is a minor point, but don't use define to set constants. You should instead use something like const int NUM_ROWS = 3; which provides type safety. –  seand Feb 28 '12 at 22:43
    
so when shld you use define? any examples? –  lakesh Feb 29 '12 at 6:07
    
@lakesh You should only use #define for preprocessor uses, like #define DEBUG, though this is often done at a higher level, i.e. in the IDE/preprocessor. –  Zeta Two Feb 29 '12 at 10:51
    
thanks.. learnt something new.... –  lakesh Feb 29 '12 at 11:13

2 Answers 2

Your random_rearrange_num function has a bias. If you want an equal chance of getting every permutation, implement something like the Fisher-Yates shuffle. Your implementation is actually a common implementation error. You can read about it here.

I would implement it as follows: (my C is mighty rusty and I don't have a compiler on hand to verify syntax or correctness)

void random_rearrange_num(int p[][numCols])
{
  int r, k, l, temp;
  for (int i = numRows * numCols - 1; i > 1; i--)
  {
    r = rand() % i; //select from only the remaining numbers
    k = r / numRows;
    l = r % numRows;
    temp = p[i][j]; 
    p[i][j] = p[k][l];
    p[k][l] = temp;
  }
}
share|improve this answer
  • Your indentation and whitespace is inconsistent. It looks like you're mostly using four-space indentation, and as such, it should be applied everywhere.

  • Your function-naming can be improved:

    • display() can be renamed to specify what it displays.
    • start() can be renamed to reflect what it is actually doing. It may be performing some initial task, but it's not quite clear from the name alone.
    • The name back_to_array() sounds a little strange and the functionality doesn't seem to help clarify its intent.
  • The stray printf("\n"); calls are a little cumbersome. You can just add the "\n" to the very start of the printf() calls that follow. It will still be clear that you're trying to start a newline.

  • It doesn't look like row_select needs to both modify its parameter and then return a new value. You could just read into a local variable and then just return that.

    Also, it appears that scanf_s is Microsoft-specific thus is non-portable. The portable version would be scanf(). However, as you have already stdafx.h included, it's already clear that this code depends on a Microsoft environment. For you, this would only be an issue if you'd like to share this code with someone else with a different environment.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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