プログラムは実行されません - 本の計算機の手紙の頻度

Question

私は単語の頻度を見つけようとしていますが、なぜプログラムがクラッシュしているのかわかりません...私はまだまだ多くの方法を試みました。私は何か間違っているのですか？ *入力は、ブックファイルと作成したい新しいファイルです。

#include <stdio.h> 
#include <stdlib.h> 
#include <string.h> 

int main(int argc, char *argv[]) 
{ 
    FILE *fp,*fp2; 
    int ch, total, counter, totalcounter, i; 
    int letters[25], letterfrequency[25]; 
    for(i=0; i<25; i++) 
    { 
     letters[i] = 0; 
     letterfrequency[i] = 0; 
    } 
    printf("Opening: %s", argv[1]); 
    fp = fopen(argv[1], "r"); 
    if (!fp) 
     { 
      perror("fopen"); 
      exit(1); 
     } 
    while((ch=fgetc(fp)) != EOF) 
     { 
      if((ch >= 'a' && ch <= 'z') || (ch >= 'A' && ch <= 'Z')) 
       { 
        counter = toupper(ch); 
        ch = counter - 65; 
        letters[ch]++; 
        totalcounter++; 
       } 
     } 

    fp2 = fopen(argv[2], "w"); 
    for(i=0;i<25;i++) 
    { 
     fprintf(fp2, "%c: Times used: %s\tFrequency Used: %s", i+65, letters[i], letters[i]/totalcounter); 
    } 
    fclose(fp); 
    fclose(fp2); 
    return 0; 
} 

Answer 1

コードにはいくつかの問題があります。まず、警告があることです。これらの警告は問題を指しています。残念ながら、ほとんどのCコンパイラはデフォルトで警告を表示しません。 -Wallでそれらを有効にする必要があります。しかし、-Wallは「すべての警告」を意味するわけではありません。理由を尋ねると、短い答えは、Cが何十年もの間、疑わしいデザインの選択肢の蓄積であり、それに慣れているということです。 ： - /私はさらに多くの警告とチェックを実行します：-Wall -Wwrite-strings -Wextra -Wconversion -std=c99 -pedantic -g。

未使用変数と初期化されていない変数があり、printfの指定が間違っていて、toupperのincludeがありません。私はそれをすべて修正するためにあなたを残します。

---

その後、次の問題は、あなたが25長いことlettersを初期化しましたが、アルファベットは26個の文字を持っています。幸いにもあなたはまたそれらを25回繰り返すだけですが、それはあなたが「Z」を失うことを意味します。これは簡単な間違いです。 26アイテムの長さの配列は0から25になりますが、その長さは26です。

配列全体の長さを繰り返すのではなく、1つの場所で配列を繰り返すのではなく、1か所で定義する方が良いでしょう。

#define NUM_LETTERS 26 

次に、配列を初期化するためのより簡単で簡単な方法があります。

int letters[NUM_LETTERS] = {0}; 

すべての要素を指定する必要はありません。残りの要素は最後の要素で埋めます。

---

それは文字ではない何かにtoupperを呼び出すために、完全に安全ですので、それはちょうどあなたがあなたのwhileループを簡素化することができ、そのまま文字を返します。

while((ch=fgetc(fp)) != EOF) { 
    ch = toupper(ch); 
    if('A' <= ch && ch <= 'Z') { 
     ch -= 65; 
     letters[ch]++; 
     totalcounter++; 
    } 
} 

ご使用のスタイルは、'A' <= ch && ch <= 'Z'です。これにより、AからZの範囲内のchのチェックであることがわかりやすくなります。

---

次の問題はletters[i]/totalcounterです。

2つの整数を分けると、整数が得られます。つまり、20/100は0です。小数点が必要な場合は、浮動小数点型の変数の1つを(double)letters[i]/totalcounter)にキャストする必要があります。

---

何の文字がファイルから読み込まれませんでしたので、letters[i]/totalcounterがゼロ除算エラーが発生します場合totalcounterが0である可能性があります。だからあなたはその事件をチェックしなければならない。あなたはユーザーがあなたにfilename引数を与えたかどうかを確認するために失敗し

if(totalcounter != 0) { 
    for(i=0; i<NUM_LETTERS; i++) { 
     printf("%c: Times used: %d\tFrequency Used: %f\n", 
       i+65, letters[i], (double)letters[i]/totalcounter 
     ); 
    } 
} 
else { 
    printf("No letters found.\n"); 
} 

---

。そうしないと、プログラムがクラッシュします。使用確認を追加することは重要です。

if(argc < 2) { 
    fprintf(stderr, "Usage: %s <filename>\n", argv[0]); 
    exit(1); 
} 

---

そして、私は結果をファイルに書いていないよ、と私は一つだけのファイル、読むために1を私のバージョンを取っていることに注意してください。私のバージョンはstdoutに出力されます。一般的に

、それがファイルに比べて標準出力にプログラム結果を印刷する方が良いでしょう。これにより、シェル配管でプログラムをうまく動作させることができ、シェル配管の柔軟性が向上します。

./wordcount somefile      # output to the screen 
./wordcount somefile.txt > somefile.count # output to a file 
./wordcount somefile.txt | program  # output to another program 

Answer 2

は、すべての文字を取るために、あなたの配列を変更

int letters[26];、すなわち、他のletterfrequenceあなたは何

に使用していないようですあなたのfprintf書式指定子が間違っている

あなたは

fprintf(fp2, "%c: Times used: %s\tFrequency Used: %s", i+65, letters[i], letters[i]/totalcounter);

を持っています

でもlettersはint配列なので対応していますCT指定子は、あなたがそう0除算が発生する可能性がtotalcounterまたはいくつかの他の任意の部門を初期化していない%d（int型）とない%s（文字列）

でなければなりません。

がヘッダーctype.hにあり、そのヘッダーがありません。