1

私はこの演習を行っていて、私の答えが正しいかどうか知りたいと思っていました。

入力内のさまざまな文字の頻度のヒストグラムを印刷するプログラムを作成します。

私は他のいくつかの答えをオンラインで見ましたが、それらは私のものとはかなり異なります。また、私のコードの形式に問題がある場合、または行われるべき改善がある場合。どんな提案でも大歓迎です。質問にはヒストグラムが必要であることは理解していますが、必要なデータがあれば、ヒストグラムを作成するのは非常に簡単です。

#include <stdio.h>
int main(){

    int userInput;
    int arrayStuff[92];
    int i, j;

    for(i = 0; i < 92; ++i){
        arrayStuff[i] = 0;
    }

    while((userInput = getchar()) != '\n'){
        if(userInput >= 30 && userInput <= 122){
            if(userInput != '\n'){
                ++arrayStuff[(userInput-30)];
            }

            if(userInput == '\n'){
                break;
            }
        }
    }

    printf("Case\t|\tOccurances\n");

    for(i = 0; i < 92; ++i){
        printf("%c\t|\t%d\n", (i+30), arrayStuff[i]);
    }
}
4

2 に答える 2

1

私が行う改善：

  • 一度に宣言して初期化int arrayStuff[92] = { 0 };し、forループを取り除きます。これにより、すべての要素が0に設定されることが保証されます。
  • さまざまな場所でマジックナンバー92を使用せず、(sizeof arrayStuff/sizeof arrayStuff[0])代わりにarrayStuffの要素数を計算するために使用します。
  • （i + 30）の括弧は冗長です
  • ユーザーが文字を入力するとどうなりますEOFか？永遠にループしているように見えます。
  • 「オカレンス」は私の辞書にはありませんが、オカレンスはあります。たぶん、文字の頻度はより良い用語ですか？
  • arrayStuff？明確で簡潔な識別子の命名の重要性を強調しすぎることはできません。頻度が頭に浮かぶ。
  • である必要がありint main (void)ますreturn 0;。これはC++ではなく、空のパラメータリストはvoidと同等です。これはCです。ここで、空のパラメーターリストは、「不明ですが固定数の引数に対する古いスタイルのK＆Rパラメーターリストです」を意味します。
于 2012-09-12T07:17:50.533 に答える
0

Jensは、92の「マジックナンバー」について言及しています。マジックナンバーは、コード内の文字通りの数字です。定数変数名、マクロ名、計算に置き換えると、値の目的や意味がわかりやすくなるため、避けることをお勧めします。30と122も「マジックナンバー」と見なされると思います。このコードは、グラフィック以外の文字を入力すると終了します。つまり、タブが入力されると停止し、タブ文字はカウントされないため、正しくない可能性があります。

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

#define FIRST_GRAPHIC_CHAR 32
#define LAST_GRAPHIC_CHAR 126
#define NUM_GRAPHIC_CHARS (LAST_GRAPHIC_CHAR - FIRST_GRAPHIC_CHAR + 1)


int main(void)
{

    int userInput;
    int arrayStuff[NUM_GRAPHIC_CHARS] = {0};
    int i;

    printf("enter a string of characters followed by <return>\n");

    userInput = getchar();
    while( userInput >= FIRST_GRAPHIC_CHAR && userInput <= LAST_GRAPHIC_CHAR )
    {
       ++arrayStuff[(userInput - FIRST_GRAPHIC_CHAR)];
       userInput = getchar();
    }

    printf("Case\t|\tOccurances\n");

    for(i = 0; i < NUM_GRAPHIC_CHARS; ++i)
    {
        if (  i + FIRST_GRAPHIC_CHAR == ' ' )
           printf("<spc>\t|\t%d\n",  arrayStuff[i]);
        else
           printf("%c\t|\t%d\n", (i + FIRST_GRAPHIC_CHAR), arrayStuff[i]);
    }
    return EXIT_SUCCESS;
}
于 2012-09-12T09:20:55.963 に答える