0

成績を含む配列を指定すると、成績の度数分布を含む小さな配列を返す computeFrequencies という関数を追加する必要があります。（これはプログラム全体のほんの一部です）

私はこれを作成しましたが、私はcに完全に慣れていないため、何が間違っていたのかわかりません: 15:29: 注: 'grades' の以前の定義はここにありました。

誰でも私を助けることができますか？どうもありがとう

void computeFrequencies(int grades[], int freq[10]){
int i, grades[];
int length=100;

for(i=0; i<length; i++){
 grades[i]=i;
 switch(i){
case 1: freq[1]++;
break;
case 2: freq[2]++;
break;
case 3: freq[3]++;
break;
case 4: freq[4]++;
break;
case 5: freq[5]++;
break;
case 6: freq[6]++;
break;
case 7: freq[7]++;
break;
case 8: freq[8]++;
break;
case 9: freq[9]++;
break;
default: freq[10]++;
}
}
}

回答ありがとうございます。エラーがなくなったにもかかわらず、プログラムが動作しません。私のプログラムは、特定の成績の度数のヒストグラムを表示する必要があります。誰でも私を助けることができますか？

入力ファイルは 1.in と呼ばれ、次の内容が含まれます。

./a.out < 1.in を使用して実行します

出力は次のようになります。

. . . * . * . . . .
. . . * . * . * . .
. . * * . * * * * .
. * * * . * * * * *
* * * * * * * * * *
1 2 3 4 5 6 7 8 9 10

コード：

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

int *readGrades() {
int x, count;
    scanf("%d", &count);
int *grades = malloc(count * sizeof(int));
for (x = 0; x < count; ++x) {
    scanf("%d", &grades[x]);
}
return grades;
}

void computeFrequencies(int grades[], int freq[10]){
  int i;
   int length=100;

   for(i=0; i<length; i++){
     grades[i]=i;
     switch(i){
    case 1: freq[1]++;
    break;
    case 2: freq[2]++;
    break;
    case 3: freq[3]++;
    break;
    case 4: freq[4]++;
    break;
    case 5: freq[5]++;
    break;
    case 6: freq[6]++;
    break;
    case 7: freq[7]++;
    break;
    case 8: freq[8]++;
    break;
    case 9: freq[9]++;
    break;
    default: freq[10]++;
    }
  }
}

int arrayMax(int length, int arr[]) {
  int i, max = arr[0];
  for (i=1; i < length; i++) {
    if (arr[i] > max) {
      max = arr[i];
    }
  }
  return max;
}

void printHistogram(int freq[10]){
  int highestGrade = arrayMax(10,freq);
  int x;
  int y;

  for(x=highestGrade; x>0; x--) {
    for(y=1; y<=10; y++) {
      if(freq[y] < highestGrade && x > freq[y]) {
    if(y==10) {
      printf(".\n");
    }
    else {
      printf(". ");
    } 
      } else {
    if(freq[y] <= highestGrade && x <= freq[y]) {
      if(y==10) {
    printf("*\n");
      }
      else {
        printf("* ");
      }   
    }
      }
    }
  }
  printf("\n");
  printf("1 2 3 4 5 6 7 8 9 10\n");
}   



int main(int argc, char *argv[]) {
  int *grades;
  int frequencies[10];

  grades = readGrades();

  computeFrequencies(grades, frequencies);
  arrayMax(10,frequencies);
  printHistogram(frequencies);

  return 0;
}
4

2 に答える 2

1

あなたのコードにはいくつかの問題があり、出現順に説明します。

まず第一に、意味が文脈からすぐには明らかではない「マジックナンバー」定数を取り除きます (とにかく、実際には定数ではないことがよくあります)。たとえば、ヒストグラムのバケット数 ( 10)。そのため、インクルードの後に​​プリプロセッサ マクロを使用して定義できるシンボリック定数 NFREQ を使用して、数字の 10 を参照し#define NFREQ 10ます。

もう 1 つのマジック ナンバーは100、周波数を計算する関数にあります。引数として渡さない限り、数値が正しいことを確認する方法はありません。この問題はループで使用され、メモリへのアクセスを保護するため、正確性に影響します。値が低すぎると、一部のデータが失われます。逆に、値が高すぎると、配列の末尾を超えて読み取ることによって未定義の動作が発生します (多くの場合、セグメンテーション違反として現れます)。この機能については後ほど説明します。まずは見てみましょうreadGrades()。

これは、データを読み取り、入力を格納する関数です。countこれは、実行時にパラメーターを取得する場所です(length上記で呼び出しました)。値を返すときに、カウントも返す方法を見つける必要があります。値を保持する変数にアドレスを渡すことができます。または、データ (グレード) と長さの両方を保持する構造体を定義し、動的に割り当てられて埋められた構造体を返すこともできます。int *readGrades(int* cnt)したがって、署名は、設定した関数のように見える可能性があります*cnt = count。

次に、computeFrequencies()余分な長さの引数を取るようになりました。freq 配列の長さを別の引数として渡すことで、関数をより汎用的にすることができます。

void computeFrequencies(int grades[], int freq[], int grades_len){
    int i;

    memset(freq, 0, sizeof(freq[0])*NFREQ); // (!) init freq array

    for (i = 0; i < grades_len; ++i) {
        freq[grades[i]-1]++;
    }
}

freq 配列を初期化することを忘れないでください。このバグは、単純な printf ループを使用して中間結果を出力する場合、またはデバッガーで値を検査する場合に簡単に見つけることができます。ここでは、グレードが範囲内[1..10]であり、グレードの総数が比較的少ないと仮定します。そうでない場合は、y 軸に沿って最大値に正規化する必要があります。ここでは、簡単なケースとして、成績配列の各成績に対応するバケット内のカウンターを増やすだけです。

さらに、コードをより堅牢にするために、ユーザー入力を検証するコードを追加できます (数字の代わりに文字を入力するとどうなりますか?)。mallocまた、が返されないことを確認する必要がありますNULL。その場合、プログラムは十分なメモリを割り当てることができず、exit誤った動作を示す戻り値 ( などEXIT_FAILURE) を返し、適切なエラー メッセージを出力してユーザーに通知する必要があります。

arrayMax()指定された長さが配列の実際の長さと一致する場合、関数は正常に動作するようです (代わりにセンチネル値を使用できます)。最も一般的な等級の最高度数を返します。

ヒストグラムを印刷する機能は、最初に修正して、いくらか単純化することもできます。1 つの問題は、悪名高い範囲外アクセスです (グレード 10 の頻度は に保存されfreq[9]ます)。C の配列インデックスは 0 から始まることに注意してください (したがってy < NFREQであり、 ではありません<=)。コードの残りの部分は自己文書化されています。

void printHistogram(int freq[]){
  int x, y, highestGrade = arrayMax(NFREQ, freq); 

  for (x = highestGrade; x > 0; --x) {
      for (y = 0; y < NFREQ; ++y) { 
          if (freq[y] >= x && x <= freq[y])
              printf("* ");
          else
              printf(". ");
          if (y == NFREQ-1)
              printf("\n");
      }
  }
  printf("1 2 3 4 5 6 7 8 9 10\n");
}

最後に、以前に動的に割り当てたメモリが不要になったらすぐに ( の return ステートメントの前にmain) を呼び出して明示的に解放することで、規律と意識を示すことができますfree(grades);。あなたの場合、メイン関数が戻ると暗黙的に解放されるため、実際には問題ではありません。

于 2013-10-03T01:34:12.937 に答える
1

再宣言を削除します。

int i, grades[];
      ^^^^^^^^^
  • 補足：関数宣言int freq[10]では、それは同等であるため誤解を招く可能性がありますint freq[]
  • 2番目の注意:freq実際に10要素の長freq[10]さが範囲外の場合
  • 3番目の注意:switchは役に立たない -i境界内にあることを確認した後、直接使用できます
于 2013-10-02T21:15:46.997 に答える