2012-03-24 15 views
0

おはようございます! これは、見通しの中でそれらの驚くほど簡単な質問の1つになってしまうだろうが、私の人生にとって私は困惑している。私はThe C Programming Languageの演習のいくつかを行っています。ループを初期化するコードを書くことができました。いくつかのグーグルの後、私は0にループを初期化するより良い方法を見つけましたが、私はそれを行うために書いたループが終了しない理由を理解していません。私はデバッガを使って、 'c'変数が決して50に達していないこと、49に達してから0にロールオーバーすることを知っていますが、なぜそれが転がっているのか分かりません。コードは下に添付されています、誰でもここで何が起こっているのか分かりますか?Cループは終了しません

#include <stdio.h> 
#define IN 1 
#define OUT 0 

/* Write a program to print a histogram of the lengths of words in 
    itsinput. */ 
main() 
{ 
    int c=0; 
    int histogram[50]={0} 
    int current_length=0; 
    int state=OUT; 

    //Here we borrow C so we don't have to use i 
    printf("Initializing...\n"); 
    while(c<51){ 
     histogram[c] =0; 
     c=c+1; 
    } 
    c=0; 
    printf("Done\n"); 

    while((c=getchar()) != EOF){ 
     if((c==32 || c==10) && state==IN){ 
      //End of word 
      state=OUT; 
      histogram[current_length++]; 
     }else if((c>=33 && c<=126) && state==OUT){ 
      //Start of word 
      state=IN; 
      current_length=0; 
     }else if((c>=33 && c<=126) && state==IN){ 
      //In a word 
      current_length++; 
     } else { 
      //Not in a word 
      //Example, " " or " \n " 
      ; 
     } 
    } 

    //Print the histogram 
    //Recycle current_length to hold the length of the longest word 
    //Find longest word 
    for(c=0; c<50; c++){ 
     if(c>histogram[c]) 
      current_length=histogram[c]; 
    } 
    for(c=current_length; c>=0; c--){ 
     for(state=0; state<=50; state++){ 
      if(histogram[c]>=current_length) 
       printf("_"); 
      else 
       printf(" "); 
     } 
    } 
} 
+1

この行に注意してください: 'while(c <51)'! – Till

+1

int histogram [50] = {0} '宣言の後にセミコロンがありません。宣言でヒストグラムを初期化したので、ループ内で再度実行する必要はありません。ループでは、 'c <51'ではなく' c <50'をチェックする必要があります。 –

+0

変数をリサイクルしないでください。できる限りローカル(小)としてスコープを作成し、必要に応じて新しいスコープを宣言します。コンパイラはこれをとにかく最適化します。 – bitmask

答えて

5

histogram[c] = 0histogramメモリc = 50を過ぎて書き込むためにです。だから、基本的にhistogram[50]は、cを上書きし、それをだから、50要素配列内の最後の有効なインデックスが49ある配列がCで0から開始するので、この問題が発生した0

になります。

技術的には、興味深く悪用できるものの、これに頼ることはできません。これは未定義の振る舞いの現れです。メモリは簡単に別のレイアウトを持っているので、「うまく動作する」か、よりうまく動作します。

1

histogramは50個の要素があります。インデックス0からインデックス49
にあなたはすべてのベットは

while (c < 50) 

または、魔法を回避するための操作を行うOFF

AREインデックス50 への書き込みを試みます定数

while (c < sizeof histogram/sizeof *histogram) 
+0

すべての定数が魔法であるとは限りません。単純な 'const int histogram_size = 50;'も同様に働き、(IMO)は 'sizeof histogram/sizeof * histogram'より読みやすくなります。 – Caleb

+0

@Caleb:Cでは、 'const int'定義は定数ではなく、読み取り専用オブジェクトを作成します。時にはあなたは本当に定数が必要な場合があります: 'sizeof arr/sizeof * arr'はそのような定数です。 – pmg

+0

どちらの場合も同じ番号が得られます。 sizeof mathを使用する場合は、配列のサイズを指定する必要があります。だから、値に名前をつけて 'int histogram [histogram_size];と'ヒストグラムを宣言するか、 'int histogram [50];と言うことができます。私は前者があなたが提起した「魔法の不変」問題をよりよく解決すると思う。 – Caleb

0

ヒストグラム内の要素0〜50にアクセスしています。ヒストグラムには要素0〜49のみが含まれています(C/C++ではゼロインデックスが使用されるため、配列の最大要素は常にサイズ1になります)。このようなエラーを回避するために

、あなたは定数として、ヒストグラムの大きさを定義し、ヒストグラムアレイに関連するすべての操作のためにそれを使用することができます。

#define HISTOGRAM_SIZE 50 

それとも(しかC99やC++のために働く、以下を参照してくださいコメント):次に

const int HISTOGRAM_SIZE = 50; 

int histogram[HISTOGRAM_SIZE]; 

そして:

while(c<HISTOGRAM_SIZE) 

'#define'はCプリプロセッサステートメントであり、コンパイル前に処理されます。コンパイラには、HISTOGRAM_SIZEが使用されているどこでも50を書いたかのように見えるので、余分なオーバーヘッドはありません。

'const int'は、多くの場合、定義と同じ結果を与える(私は100%特定の状況ではないが、他のものは詳しく述べることはできません)あなたにタイプチェックの追加ボーナスを与えます。

+1

C89では、 'const int'定義は配列次元で使用可能な定数を定義しません(読み込み専用オブジェクトを定義します)。 C99では、定数を必要としないVLA(Variable Length Arrays)を使用してサイズを指定できます。 – pmg

+0

ああ、ありがとう!私はそれを反映するために上記を編集しました。 – sonicwave

関連する問題