a C program about 8 empresses question

naihe2010 · 10-24-2005, 02:34 AM

I checkde the program manytimes,and can't fix it.Please help me .

Code:

#include <stdio.h>

#define N 8

int h[N],l[2*N+1],r[2*N],pla[N],ok;

int main(void){
  int i,k,temp;
  for(i=1;i<=N;h[i++]=1);
  for(i=1;i<=2*N;l[i]=1,r[i++]=1);
  ok=1;
  i=1;
  pla[1]=1;
  while(i<=N && i>0){
    if(ok){
      if(i==N){
	for(k=1;k<=N;++k){
	  temp=pla[k];
	  printf("%d:%d\n",k,temp);
	}
	printf("\n\n\n");
      }
      else{
        h[pla[i]]=l[i-pla[i]+N]=r[i+pla[i]]=0;
	pla[++i]=1;
      }
    }
    else{
      if(pla[i]==N){
	--i;
	h[pla[i]]=l[i-pla[i]+N]=r[i+pla[i]]=1;
      }
      if(pla[i]<N)++pla[i];
    } 
    ok=h[pla[i]] && l[i-pla[i]+N] && r[i+pla[i]];
  }
  return 0;
}

Orkie · 10-24-2005, 04:18 AM

What exactly is is supposed the do? I would also try spacing it out a bit more - that solid lump of code is very hard to read.

csst0136 · 10-24-2005, 05:01 AM

whatever the program does i think its a very bad writing program especially when u r using FOR its very bad.i think u have to rewrite the program again using FOR.For example u have to use the first like this
for (i=0;i<=N;i++)
{
H[i]=1;
}
if u run a simple program with only command
for(i=1;i<=N;h[i++]=1);
maybe u dont get errors but it doesnt work.Its very very very bad using H[i++]=1;

perfect_circle · 10-24-2005, 07:11 AM

Quote:

Originally posted by csst0136
whatever the program does i think its a very bad writing program especially when u r using FOR its very bad.i think u have to rewrite the program again using FOR.For example u have to use the first like this
for (i=0;i<=N;i++)
{
H[i]=1;
}
if u run a simple program with only command
for(i=1;i<=N;h[i++]=1);
maybe u dont get errors but it doesnt work.Its very very very bad using H[i++]=1;

Why?

dmail · 10-24-2005, 07:32 AM

Quote:

Originally posted by csst0136
for(i=1;i<=N;h[i++]=1);
maybe u dont get errors but it doesnt work.Its very very very bad using H[i++]=1; [/B]

it may not be the nicest thing to look at, or help when you are trying to debug, but this is valid, if you don't want h[0] to be intailised.

edit--
you start your loop at 1, is this intended?
your i never gets past 6 so that "if(i==N)" is never true (well not in the time i have ran it). and comments are wonderful things, even if its to let other people know whats going on.

tristanm · 10-24-2005, 10:05 AM

Quote:

Originally posted by perfect_circle
Why?

Well you should always write your code with other people in mind. It is very seldom that you will be the only one who ever looks at your code and while it may make perfect sense to you, when someone else tries to fix a bug or maintain the code later on it will be a nightmare for them.

Tristan

Dark_Helmet · 10-24-2005, 12:25 PM

I don't have an answer (aside from a few observations), but will re-post your code with my comments inside it. Note: I also changed some of the code layout to match my preferred style. It doesn't mean your original style is wrong, but this format is the way I'm most comfortable working with code.

Code:

#include <stdio.h>

#define N 8

/* Why create variables here?? You don't have any functions to call. There's
   no need to make global variables, especially considering you declare
   variables inside main() - which is the standard way of doing things.

   On another note, single letter variables are bad, bad, bad. Use 
   *meaningful* variable names for two reasons:
   1. It makes the code easier to understand. For instance, h is declared
      as an integer array. Ok great, but what does it represent? A game board?
      A set of playing piece ID numbers? A history of moves made? 'h' tells
      the reader absolutely nothing about what the variable's purpose is.
   2. Say you work on your code and you realize you want to change the
      variable's name. Let's look at the variable 'l' for this one. It would be
      a nightmare to use search-and-replace. Replacing the 'l' would screw up
      the "while" and "else" keywords in addition to the "pla" variable, and
      any other item with an 'l' in it. Similarly, if you want to see where
      a variable is used, searching for 'l' will take forever to find what
      you're really interested in.

      And if you decide to rename the variable without using search-and-replace
      by manually hunting down each reference, you're not guaranteed to find
      each one (because a single-letter variable is hard to spot), and you'll
      spend your time compiling-fixing-compiling-fixing-compiling-... because
      the compiler will only complain for the *first* time an undeclared
      variable is used. It will not list them all. */
int h[N];
int l[2*N+1];
int r[2*N];
int pla[N];
int ok;

int main( void )
{
  int i;
  int k
  int temp;

  /* Initialization (?) h[1] through h[N-1] equal 1
     Incidentally, this should have caused an error. h[N] is not a valid
     array location and should have caused a segmentation fault. 

     Also, h[0] contains junk - it's uninitialized. */
  for( i = 1; i <= N; h[i++] = 1 );


  /* Another initialization? Although this one tries to do two initializations
     at once. Again, you should have received a segmentation fault when
     trying to assign "r[2*N] = 1". And also as above, both l[0] and r[0] are left
     uninitialized, and contain junk.

     You could combine all of them into one cleaner for loop:
       for( i = 0; i < N; i++ )
       {
         h[i]       = 1;
         l[2*i]     = 1;
         l[2*i + 1] = 1;
         r[2*i]     = 1;
         r[2*i + 1] = 1;
       }                                                     */
  for( i = 1; i <= 2 * N; l[i] = 1, r[i++] = 1 );


  /* Setting flags to known state */
  ok=1;
  i=1;
  pla[1]=1;


  /* I would suggest using parentheses around logical expressions to guarantee
     you don't have any problems in the order the compiler performs its
     operations.

     Also, you hide your increment of i in a "pla[++i] = 1" statement but you
     later pull out the decrement altogether: "--i". Incrementing i is
     important to the operation of this while loop - don't hide it in an
     expression that's doing something else. Pull it out, and follow the
     same format you did with "--i". */
  while( ( i <= N ) && ( i > 0 ) )
  {
    if( ok )
    {
      if( i == N )
      {
	for( k = 1; k <= N; ++k)
        {
	  temp = pla[k];
	  printf( "%d:%d\n", k, temp );
	}
	printf("\n\n\n");
      }
      else
      {
        /* I have absolutely NO IDEA what this complicated behemoth
           statement is trying to accomplish. */
        h[pla[i]] = l[i-pla[i]+N] = r[i+pla[i]] = 0;
	pla[++i] = 1;
      }
    }
    else
    {
      if( pla[i] == N )
      {
	--i;
        /* I have absolutely NO IDEA what this complicated behemoth
           statement is trying to accomplish. */
	h[pla[i]] = l[i-pla[i]+N] = r[i+pla[i]]=1;
      }

      /* This if statement (as originally typed) was basically hiding
         what it was doing. Don't stick functional code on the same line
         following an if. Any readers, including yourself, will have a
         tendency to skip over the line with the if to see what the code
         does when it's satisfied. At first glance, I thought it was an
         empty if statement that did nothing at all. */
      if( pla[i] < N )
      {
        ++pla[i];
      }

    } 

    /* Again, complicated statement where the program is referring to array
       locations based on values in other variables without any sort of
       meaningful name attached to any of them. I get dizzy just looking at
       it, and give up trying to understand what it's doing very quickly.

       Don't make people *work* to understand your code - especially when
       you're looking for assistance. */
    ok = h[pla[i]] && l[i-pla[i]+N] && r[i+pla[i]];
  }

  return 0;
}

perfect_circle · 10-24-2005, 03:38 PM

Quote:

Originally posted by trmartindale
Well you should always write your code with other people in mind. It is very seldom that you will be the only one who ever looks at your code and while it may make perfect sense to you, when someone else tries to fix a bug or maintain the code later on it will be a nightmare for them.

Tristan

The "Why?" was because this kind of code makes perfect sence to me. I don't see why this is bad.
In "The C programming language" by Kernighan & Ritchie, the developers of C, you'll see a lot of string[counter++].

In the linux kernel source tree, do a:

Code:

grep -R "++\]" * |grep "for"

and you'll find:

Code:

drivers/scsi/wd7000.c:  for (i = 0; i < UNITS; wd7000_host[i++] = NULL);
drivers/scsi/wd7000.c:  for (i = 0; i < NUM_CONFIGS; biosptr[i++] = -1);
drivers/cdrom/sjcd.c:   for (i = 0; i < SJCD_BUF_SIZ; sjcd_buf_bn[i++] = -1);
drivers/s390/crypto/z90main.c:  for (chkdom = 0; chkdom <= 15; cdx_array[chkdom++] = -1);

dmail · 10-24-2005, 03:46 PM

Quote:

Originally posted by perfect_circle
...this kind of code makes perfect sence to me. I don't see why this is bad...

why don' t you help naihe2010 sort the problem out then lmao. i looked the code and just couldn't be bothered to figure it out.

perfect_circle · 10-24-2005, 08:29 PM

the obvious error is that he seems to think that h[N] is h[1 2 ........N] and not h[0 1 ........N-1].
I couldn't determine either what he is trying to do. What I was saying is that h[i++] is not the problem.

King of Men · 10-25-2005, 02:29 PM

I refuse to read this kind of code, but in an effort to be helpful to others, I'll point out that 'Eight empresses' probably refers to the eight queens problem in chess : Place eight queens on a chessboard so none of them threaten each other. Presumably the code is intended to generate a solution. I have absolutely no idea how it does that, though.

Orkie · 10-25-2005, 02:51 PM

I can see how that would be true -- N is 8, the number of squares along one edge of a chess board. How you figured that out, I do not know

naihe2010 · 10-25-2005, 09:04 PM

Yes ,it should be 8 queens question. I made a mistake.
Now I fixed the code ,and it can work well . please give your advise to me .

Code:

#include <stdio.h>

#define N 8

int h[N+1],l[2*N+1],r[2*N+1],pla[N+1],ok;

int main(void){
  int i,k,temp;char ano;
  for(i=1;i<=N;h[i++]=1);
  for(i=1;i<=2*N;l[i]=1,r[i++]=1);
  ok=1;
  i=1;
  pla[1]=1;
  while(i<=N && i>0){
    if(ok){
      if(i==N){
	for(k=1;k<=N;++k){
	  temp=pla[k];
	  printf("%d:%d\n",k,temp);
	}
	printf("\n\n\n");
                printf ("Need another answer ? 'Q' or 'q' to quit!");
                scanf("%c",&ano);
                if(ano=='q' || ano=='Q')exit(0);
                --i;
                h[pla[i]]=l[i-pla[i]+N]=r[i+pla[i]]=1;
                ++pla[i];
      }
      else{
        h[pla[i]]=l[i-pla[i]+N]=r[i+pla[i]]=0;
	pla[++i]=1;
      }
    }
    else{
      if(pla[i]>=N){
	--i;
	h[pla[i]]=l[i-pla[i]+N]=r[i+pla[i]]=1;
      }
      ++pla[i];
    } 
    ok=h[pla[i]] && l[i-pla[i]+N] && r[i+pla[i]];
  }
  return 0;
}

bulliver · 10-26-2005, 02:10 AM

I would replace:

Code:

scanf("%c",&ano);

with:

Code:

do {
ano = getchar();
}
while (ano == '\n');

as this will clear up the newline problem...
Also, it won't compile because "ano" is not declared. I just added "char ano;"

naihe2010 · 10-26-2005, 04:22 AM

Thanks very much .
For a long time,I want to find the difference between "scanf" and "getchar" when i input a char. And ,your code give me a examble . I thinks a little,but not clearly.
Would you tell their details to me more?