プログラムを見やすく改良したい

Question

正常に動くプルグラムを見やすく改良したい。
具体的に教えていただければありがたいです。セグメンテーションフォルトでベスト7まで表示して停止します。173行あたりだと思うのですが、よくわかりません。
"getputch.h"をお知らせしたいのですが文字数がオーバーします。どうすれば送れますか？

コード
// ラックナンバーサーチ・トレーニング
// 過去の履歴とそれらの実行日時,実行時間
// 過去のbesttenの実行日時,実行時間
// 最近の10回の実行日時,実行時間
#include <stdio.h>
#include <time.h>
#include <float.h>
#include <ctype.h>
#include <stdlib.h>
#include <sys/types.h>
#include <unistd.h>
#include "getputch.h"

#define MAX_STAGE    3
#define swap(type, x, y)    do { type t = x; x = y; y = t; } while (0)
#define MAX_NUM         10

char dtfile[]  = "LACKNUM.DAT";

// 最高記録用のファイル
char dtfile2[] = "LACKNUM2.DAT"; 

typedef struct {
  int tm_year;
  int tm_mon;
  int tm_mday;
  int tm_hour;
  int tm_min;
  int tm_sec;
  double score;
} BEST_TEN;

// --- 比較用の関数 cmp ---
static int
cmpptr( const void *p, const void *q ) {
  return (*(BEST_TEN**)p)->score - (*(BEST_TEN**)q)->score;
}

//1回実行したとき進むポイントの大きさを求める
static int get_point_offset(fpos_t* offset) {
  FILE *fp;
  struct tm local;
  double score;

  fp=fopen(dtfile, "rb");
  if(fp==NULL) {
  	perror("fopen");
  return -1;
  }

  if(fread(&local, sizeof(struct tm), 1, fp)<0) {
    perror("fread");
    fclose(fp);
    return -1;
  }

  if(fread(&score, sizeof(double), 1, fp)<0) {
    perror("fread");
    fclose(fp);
    return -1;
  }

  //ファイルポインタのfp位置を取得
  if(fgetpos(fp, offset)!=0) {   
    perror("fgetpos");
    fclose(fp);
    return -1;
  }
	
  fclose(fp);

  //offsetの示す場所に格納します。
  printf("ファイルポインタの位置offsetは「%d」です。\n", *offset);
  
  
  return 0;
}

//local のデータを新しい順にならべる。
static void update_local_data(fpos_t ft,int k) {
  FILE *fp;         	// これまでの履歴を指すポインタ
  double score;     	//これまでの履歴の所要時間
  int i;

  struct tm local; //構造体変数 localの大きさ

  if((fp = fopen(dtfile,"rb"))==NULL) {
    printf("ファイルを作成します。\n\n");
    bestscore=DBL_MAX;  // float.hに定義されている。double型で表現できる最大値を表すマクロDBL_MAX 
    return;
  }

  printf("最近の10回の点数とそれらの実行日時は\n\n");
  for(i = ft - k; i >= ft - 10 * k; i -= k) { 	//構造体変数 localの大きさoffsetで44
    
    if(fseek(fp, i, SEEK_SET) != 0) {			//ファイルポインタを先頭からiまで移動
      perror("fseek");
      break;
    }

    if(fread(&local, sizeof(struct tm), 1, fp) < 0) {
      perror("fread");
      break;
    }
    printf("%d年 %d月 %d日 %d時 %d分 %d秒\n",
        local.tm_year + 1900, local.tm_mon + 1,
        local.tm_mday, local.tm_hour, local.tm_min, local.tm_sec);
    if(fread(&score, sizeof(double), 1, fp) < 0) {
      perror("fread");
      break;
    }
    printf("得点（所要時間）は%.1f秒\n\n", score);
  }
  fclose(fp);

  printf("enterキーで開始します。\n");
  fflush(stdout);
  getchar();
}

//過去のトレーニング情報を取得・表示して最高得点を返す 
static double  get_data(int *count) {
  FILE *fp;        		//これまでの履歴を指すポインタ 
  double score;      	//これまでの履歴の所要時間
  double bestscore;     //これまでの最短所要時間
  int i,j=0;
  
  BEST_TEN best_ten[100]={0};
  BEST_TEN *plst[*count];
  struct tm local;
  

  if((fp = fopen(dtfile, "rb"))==NULL) {
    printf("ファイルを作成します。\n\n");
    bestscore = DBL_MAX;  // float.hに定義されている。double型で表現できる最大値を表すマクロDBL_MAX 
    return 0.0;
  }

  printf("\n過去の履歴\n-------------------------- \n");

  while((i=fread(&local, sizeof(struct tm), 1, fp))>0 ){
    printf("%d年 %d月 %d日 %d時 %d分 %d秒\n",
        local.tm_year + 1900, local.tm_mon + 1,
        local.tm_mday, local.tm_hour, local.tm_min, local.tm_sec);
    fread(&score, sizeof(double), 1, fp);
    printf("得点（所要時間）は%.1f秒\n\n", score);

    best_ten[j].tm_year = local.tm_year;
    best_ten[j].tm_mon = local.tm_mon;
    best_ten[j].tm_mday = local.tm_mday;
    best_ten[j].tm_hour = local.tm_hour;
    best_ten[j].tm_min = local.tm_min;
    best_ten[j].tm_sec = local.tm_sec;
    best_ten[j].score = score;
    j++;
    (*count)++;
  }
  printf("count1は%d\n\n", *count);

  for(i=0; i < *count; i++)
    plst[i]=&best_ten[i];

  //qsort(配列名, 配列の数, 配列一つのバイト数, 比較関数)
  qsort(plst, *count, sizeof(BEST_TEN*), cmpptr);

  printf("\n過去のscoreのbestscore10個\n-------------------------- \n");

  // 並べ替え後の内容を表示
  for (i=0;i<10; i++) {
    printf("%d年 %d月 %d日 %d時 %d分 %d秒 \n"
        ,plst[i]->tm_year+1900, plst[i]->tm_mon+1, plst[i]->tm_mday,
        plst[i]->tm_hour,plst[i]->tm_min,plst[i]->tm_sec);
    printf("得点（所要時間）は %.1f秒です。\n\n", plst[i]->score);
  }

  bestscore=plst[0]->score;

  fflush(stdout);
  fclose(fp);

  printf("count2は%d\n\n", *count);
  return bestscore;
}

//今回のトレーニング情報を書き込む 
static int put_data(double score, double bestscore) {
  FILE *fp;    		//fpはfopenされたdtfileを指すポインタ
  FILE *fp2;  		//fp2はfopenされたdtfile2を指すポインタ、最短時間が格納されている
  time_t t = time(NULL);
  struct tm *local = localtime(&t);

  time(&t);
  local = localtime(&t);
  if((fp = fopen(dtfile, "ab"))==NULL) {
    printf("ファイルがありません");
    fflush(stdout);
    return -1;
  }

  if(fwrite(local, sizeof(struct tm), 1, fp)!=0) { 	
    perror("fwrite");
    fclose(fp);
    return -1;
  }

  if(fwrite(&score, sizeof(double), 1, fp)!=0) { 		
    perror("fwrite");
    fclose(fp);
    return -1;
  }

  fclose(fp);

  if(score > bestscore) {
    return 0;
  }

  if((fp2 = fopen(dtfile2, "wb"))==NULL) {
    printf("ファイルがありません");
    fflush(stdout);
    return -1;
  }

  if(fwrite(local, sizeof(struct tm), 1, fp2)!=0) {
    perror("fwrite");
    fclose(fp);
    return -1;
  }

  if (fwrite(&bestscore, sizeof(double), 1, fp2)!=0) {	//fpに書き込む
    perror("fwrite");
    fclose(fp);
    return -1;
  }
  
  fclose(fp2);
  return 0;
}

//トレーニングを実行して得点（所要時間）を返す 
static double do_training(void) {
  int i, j, stage;
  int dgt[9]={1, 2, 3, 4, 5, 6, 7, 8, 9};
  int a[8];
  double jikan;       	// 時間
  clock_t start, end; 	// 開始時刻・終了時刻

  printf("\n\n欠けている数字を入力してください。\n");
  printf("スペースキーで開始します。\n");
  while(getch()!=' ')
    ;
  start=time(NULL);

  for(stage=0; stage < MAX_STAGE; stage++) {
    int x=rand() % 9; // 0～8の乱数を発生 ,1から9までのうち8文字を表示するための数字 
    int    no;          // 読み込んだ値 ,キーボードから打ち込んだ数字 

    i=j=0;
    
    // dgt[x]を飛ばしてコピー 
    while (i<9){
      if (i!=x)
        a[j++]=dgt[i];	// dgt[x]は1から9までの数字が入っている配列 
      i++;
    }

    for(i=7; i>0; i--) {		// 配列aをシャッフル、配列の添字は0から7までの8個である 
      int j=rand()%(i + 1);		//jは1から7まで変化する。
      
      if(i!=j)
        swap(int, a[i], a[j]);		//iとjの添字でシャッフルする。 
    }

    printf("%d回目：", stage+1);

    // 全要素を表示 
    for(i=0; i<8; i++) {
      printf("%d ", a[i]);
    }
    printf("：");
    fflush(stdout);

    do{
      no=getch();
      if(isprint(no)){          // 表示可能であれば 
        putch(no);               // 押されたキーを表示 
        if(no!=dgt[x] + '0')   // 正解でなければ 
          putch('\b');           // カーソルを一つ戻す 
        else
          printf("\n");           // 改行 
      }
    }while(no!=dgt[x] + '0');
  }
  end=time(NULL);

  jikan=(double)difftime(end, start);

  printf("%.1f秒かかりました。\n", jikan);

  if(jikan>25.0)
    printf("鈍すぎます。\n");
  else if(jikan > 20.0)
    printf("少し鈍いですね。\n");
  else if(jikan > 17.0)
    printf("まあまあですね。\n");
  else
    printf("素早いですね。\n");

  return jikan;
}

int main(void) {
  int retry, count=0;  // もう一度？
  double score;          // 今回の所要時間
  double bestscore;      // 最短所要時間
  //double jikan;          // 時間
  FILE *fp;
  fpos_t ft;
  fpos_t offset;
  int k;

  // get_data()で前回までの最短所要時間をbest2に格納する。
  bestscore=get_data(&count);

  // ファイルから読み込んでbestに代入する。
  printf("bestscoreは%.1f\n\n", bestscore);
  printf("count3は%d\n\n", count);

  fp=fopen(dtfile, "rb");
  if(fp==NULL) {
    perror("fopen");
    return -1;
  }

  if (get_point_offset(&offset)!=0) {
    return -1;
  }

  k=offset;
  //ファイルポインタの位置を取得
  printf("mainのget_point_offset(&offset)後のkの値は「%d」です。\n\n", k);

  //ファイルポインタを末尾まで移動
  if (fseek(fp, 0, SEEK_END)!=0) {
    perror("fseek");
    return -1;
  }

  //ファイルポインタの位置を取得
  //ファイルfpの現在のファイル位置を取得して
  if (fgetpos(fp, &ft)!=0) {
    perror("fgetpos");
    fclose(fp);
    return -1;
  }

  // &ftの示す場所に格納します。
  update_local_data(ft, k);

  if(fgetpos(fp, &ft)!=0) {
    perror("fgetpos");
    return -1;
  }
  fclose(fp);

  //best = get_data(ft);
  // get_data()で前回までの最短所要時間を
  //ファイルから読み込んでbestに代入する。

  //ライブラリの初期処理235p、initscr(),cbreak(),noecho(),refresh()
  init_getputch();

  // 乱数の種を初期化
  srand(time(NULL));
  
  do{
    // トレーニング(do_training)で実行、返却された所要時間(jika)を
    // scoreに代入する。
    score=do_training();
    
    if(score < bestscore){
      printf("最短所要時間を更新しました!!\n");
      // 最高得点更新 
      bestscore=score;
    }

    printf("もう一度しますか … (0)いいえ (1)はい：");
    retry=0;
    scanf("%d", &retry);
  } while (retry==1);

  // 今回の日付・時刻・得点を書き込む 
  put_data(score,bestscore);
  fflush(stdout);

  //ライブラリの初期処理235p,endwin()
  term_getputch();

  return 0;

Comments

[coco_bauer]

どんな立場の人にとっての「見やすさ」なのですか？

[mattn]

ちなみに、プルグラムでなくプログラムです。

[Stripe]

#include "getputch.h"って何ですか？　コードの一部を別のソースファイルに移して、本体のソースコードを簡素化するのもアリなんですか？

[退会済みユーザー]

Stripeさんありがとうございます。おっしゃることはわかるのですが、それができなくてこまっています。struct　tmを日付とbestにわけたり、bestをわかり易い名前scoreに変更して、書きなおしてやってみたのですが、うまく分割、整理ができません。分割したときのmainもいまいちよく書けません。

[退会済みユーザー]

一応動くのですが、名前、ソースコードの簡素化、無駄なコード、エラー処理の追加などを関数ごとにテストしたいのですが、そのときのmain関数もおねがいします。

[退会済みユーザー]

今夜仕事なので、明日いただいたコードを参考に修正に挑戦したいとおもいます。ありがとうございます。いろいろ指摘いただき恥しいかぎりです。初心者で申し訳ありません。趣味で、独学です。

Answer 1

気にかかった点を列挙させてもらいます。

１）データ型に気を掛けて

// --------------- 比較用の関数 cmp -------------------
int kaime_p(fpos_t* kaime_p1)
{
  //途中のコードは割愛しました。
 
    return *kaime_p1;
}

例えば、関数kaime_pの返り値の型はintであると定義されています。返り値は、kaime_p1です(return kaime_p;)から、変数kaime_p1の型は int であるはずです。ところが、引数のkaime_p1の型宣言は fpos_t となっています。残念ながら、コードにはfpos_tの定義が含まれていません（たぶん、"getputch.h"で定義されているのでしょうが）から、どのようにすれば正しい記述になるのかが判りません。

このように型に整合性がないと、コードが判りにくくなります（私が監督者なら、コーディングミスとして差し戻すところ）
また、fpos_tという型はコードの各所で使われていますから、intと分ける必要性をちゃんと説明すべきです。fpos_tが何のための型なのかが判っていると、intとfpos_tを使い分けている箇所のプログラマの意図が察せされるのでコードの理解がしやすくなる可能性があります。

標準ではないinclude(#include <...>ではなく、#include "..."でインクルードされている）で定義されている型については、簡単で良いので、それぞれの説明をコードの初めに近い場所でしておくと読みやすいコードになります。特に名前が似た型が定義されている場合や、インクルードされているファイルが多い場合に有効です。

２）下手な英語もどきより、ローマ字で命名
英語で適切な名前を考えるのはハードルが高いですから、誰でも読めるローマ字を使いましょう。
dtfile[],dtfile2[]よりfileMei_KakoKiroku[],fileMei_saikoKiroku[]のほうがましでしょ。
go()よりkaitouJikanSokutei()のほうが、ラックナンバーサーチの回答時間を測定する関数だと判り易い でしょ。

３）バイナリファイルを使う必要がありますか？
質問のコードを読みにくくしているのは、バイナリファイルを使っている事が一因だと思います。
CSV形式のテキストファイルなら、readlineで１レコード(1行)読出して、splitで分割というように何をしているのかが判り易いですが、バイナリファイルだとsizeofで読みだすバイト数を求めて等々面倒な記述（読みにくい記述）が避けられませんし、動作が直観的ではありません。
テキストファイルからメモリに一気に読出し、メモリ上で処理して、最後に結果を書き戻すというやりかただとファイルとのやり取りをほとんど意識しなくて済みます。また、デバッグ時にファイルの内容が簡単に確認できるというメリットもあります。（バイナリファイルの内容を確認するのは、面倒です）

Answer 2

ゲームを実行する機能だけ作っても、これくらい複雑になりました。
...私のコードにも粗があるかと思います。参考程度によろしくお願いします。

/* main.c */
#include "score_datum.h"
#include "training.h"

#include <stdio.h>

int main(void) {
    SCORE_DATUM score_datum;
    do_training(&score_datum, DEFAULT_STAGE_NUM);
    print_score_datum(&score_datum);

    return 0;
}

/* score_datum.h */
#ifndef INCLUDED_SCORE_DATUM
#define INCLUDED_SCORE_DATUM

#include <time.h>

typedef struct tm TIME_DATUM;

typedef struct {
    TIME_DATUM time_datum_;
    double score_;

} SCORE_DATUM;

TIME_DATUM * get_time_datum(SCORE_DATUM *);
double get_score(SCORE_DATUM *);

void set_score_datum(SCORE_DATUM *dst, double);
void print_score_datum(SCORE_DATUM *);

#endif

/* score_datum.c */
#include "score_datum.h"

#include <stdio.h>
#include <time.h>

#define BUF_SIZE 128

TIME_DATUM * get_time_datum(SCORE_DATUM *datum) {
    return &datum->time_datum_;
}
double get_score(SCORE_DATUM *datum) {
    return datum->score_;
}

void set_score_datum(SCORE_DATUM *dst, double score) {
    time_t tmp_time;
    time(&tmp_time);

    dst->time_datum_ = *localtime(&tmp_time);
    dst->score_ = score;
}

void print_score_datum(SCORE_DATUM *datum) {
    char time_buffer[BUF_SIZE];
    strftime(
        time_buffer, BUF_SIZE,
        "%Y/%m/%d %H:%M:%S", 
        get_time_datum(datum)
    );

    printf("%s : SCORE %.1f\n", time_buffer, get_score(datum));
}

/* training.h */
#ifndef INCLUDED_TRAINING
#define INCLUDED_TRAINING

#define DEFAULT_STAGE_NUM 3
double do_training(SCORE_DATUM *result, int stage_num);

#endif

/* training.c */
#include "my_util.h"
#include "score_datum.h"
#include "training.h"

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

void do_one_stage(void);
int *make_lack_array(int [], size_t, int [], size_t, int);

#define NUM_LENGTH 9

double do_training(SCORE_DATUM *result, int stage_num) {
    clock_t start_time, end_time;
    double elapsed_time;

    srand(time(NULL));

    printf("\n\n");
    printf("Input lacked number.\n");
    printf("Press space key to start game.\n");

    wait_key_input(' ');
    start_time = clock();

    for(int i = 0; i < stage_num; i++) {
        do_one_stage();
    }

    end_time = clock();
    elapsed_time = difftime_precise(end_time, start_time);

    set_score_datum(result, elapsed_time);  
    return elapsed_time;
}
void do_one_stage(void) {
    int complete_array[NUM_LENGTH];
    for(size_t i = 0; i < NUM_LENGTH; i++) {
        complete_array[i] = i + 1;
    }

    int lacked_number = rand() % NUM_LENGTH + 1;
    int lacked_array[NUM_LENGTH - 1];

    int *result = make_lack_array(
        complete_array, ARRAY_LEN(complete_array),
        lacked_array, ARRAY_LEN(lacked_array),
        lacked_number
    );
    if(result == NULL) {
        perror("make_lack_array");
        exit(EXIT_FAILURE);
    }

    print_array(lacked_array, ARRAY_LEN(lacked_array));

    int input_num;
    do {
        input_num = input_int();

    } while(input_num != lacked_number);

    printf("Correct.\n\n");
}

//
//
int count_num_in_array(int array[], size_t size, int num) {
    int count = 0;
    for(size_t i = 0; i < size; i++) {
        if(array[i] == num) count++;
    }
    return count;
}
int *make_lack_array(int src_array[], size_t src_size,
                       int dst_array[], size_t dst_size, int lack_num) {
    
    if(src_size - 1 > dst_size) {
        return NULL;
    }
    if(count_num_in_array(src_array, src_size, lack_num) != 1) {
        return NULL;
    }

    for(size_t i = 0, j = 0; i < src_size; i++) {
        if(src_array[i] == lack_num) {
            continue;
        }
        dst_array[j] = src_array[i];
        j++;
    }
    return dst_array;
}

/* my_util.h */
#ifndef INCLUDED_MY_UTIL
#define INCLUDED_MY_UTIL

#include <stdlib.h>
#include <time.h>

#define SWAP(type, a, b)                \
    do {                                \
        type tmp = a; a = b; b = tmp;   \
    } while(0)

#define ARRAY_LEN(array)                \
    sizeof(array) / sizeof(array[0])

#define WAIT_ANY_KEY -1
void wait_key_input(char);
int input_int(void);

void shuffle_array(int [], size_t);
void print_array(int [], size_t);
double difftime_precise(time_t, time_t);

#endif

/* my_util.c */
#include "my_util.h"
#include <stdio.h>
#include <stdlib.h>
#include <time.h>

int input_int(void) {
    int ret;

    char buffer[10];
    if(fgets(buffer, sizeof(buffer), stdin) != NULL) {
        ret = atoi(buffer);
    }
    else {
        perror("input_int");
        exit(EXIT_FAILURE);
    }

    return ret;
}
void wait_key_input(char key) {
    if(key == WAIT_ANY_KEY) {
        getchar();
        return;
    }

    while(getchar() != key);
}

void shuffle_array(int array[], size_t size) {
    for(size_t i = 0; i < size; i++) {
        int rand_index = rand() % size;
        SWAP(int, array[i], array[rand_index]);
    }
}
void print_array(int array[], size_t size) {
    for(size_t i = 0; i < size; i++) {
        printf("%d ", array[i]);
    }
    printf("\n");
}
double difftime_precise(time_t time2, time_t time1) {
    return (double)(time2 - time1) / CLOCKS_PER_SEC;
}

---

実行例

Input lacked number.
Press space key to start game.

1 2 3 4 5 7 8 9
6
Correct.

1 2 3 4 5 7 8 9
6
Correct.

1 2 3 4 5 6 7 9
9
8
Correct.

2017/09/14 12:01:40 : SCORE 7.6

ランキング機能を付けると、ファイル入出力が絡むだけもっと複雑になります。

追記

だいたい同じようなゲームを、Pythonでも実装してみました。

# main.py

from score_datum import ScoreDatum
from training import do_training

def main():
    now_score_datum = do_training()
    now_score_datum.print_info()

if __name__ == '__main__':
    main()

# score_datum.py

import datetime


class ScoreDatum():
    def __init__(self, score: float):
        self._time_datum = datetime.datetime.today()
        self._score = score

    @property
    def time_datum(self) -> datetime.datetime:
        return self._time_datum

    @property
    def score(self) -> float:
        return self._score

    def print_info(self) -> None:
        print(self.time_datum.ctime(), round(self.score, ndigits=1)) 

# training.py

from score_datum import ScoreDatum
from my_util import wait_key_input
import random
import time

NUM_LENGTH = 9

def do_training(stage_num: int = 3):
    print()
    print('Input lacked number.')
    print('Press space key to start game.')
    wait_key_input(' ')

    start_time = time.time()
    for _ in range(stage_num):
        do_one_stage()

    elapsed_time =  time.time() - start_time
    return ScoreDatum(elapsed_time)

def do_one_stage() -> None:
    complete_list = list(range(NUM_LENGTH + 1))[1:]
    
    lacked_number = random.randint(1, NUM_LENGTH)
    lacked_list = _make_lack_list(complete_list, lacked_number)

    print(lacked_list)
    while True:
        try:
            input_num = int(input())
        except:
            continue

        if input_num == lacked_number:
            break

    print('Correct.')
    print()

def _count_num_in_list(src_list: list, num: int) -> int:
    return src_list.count(num)

def _make_lack_list(src_list: list, lack_num: int) -> list:
    if _count_num_in_list(src_list, lack_num) != 1:
        return None

    src_list.remove(lack_num)
    return src_list

# my_util.py

def wait_key_input(key: str = None) -> None:
    if key is None:
        input()
        return

    while input() != key:
        pass

---

実行例

Input lacked number.
Press space key to start game.

[1, 2, 4, 5, 6, 7, 8, 9]
3
Correct.

[1, 2, 3, 4, 5, 7, 8, 9]
6
Correct.

[1, 2, 3, 4, 5, 6, 8, 9]
6
7
Correct.

Thu Sep 14 13:39:56 2017 5.6

リスト処理が楽なのは嬉しいですね。
文字数でカウントすると、Cの場合の約40%で実装出来ました。

Comments

[退会済みユーザー]

LouiS0616さんありがとうございます。端末にはどのように実行するのですか。リンクの仕方がよくわかりません。随分前にやったことあるのですが、課題1個やるのに時間がかかりすぎて、前に進めません。よろしくお願いします。

[LouiS0616]

gcc main.c score_datum.c training.c my_util.c でいけますよ。

[退会済みユーザー]

LouiS0616さん実行できました。分割ファイルの実行の復習もできてありがとうございました。甚だずうずうしいお願いなのですが、今日コードの書き直しをしたところ、説明文にあるような結果になり、困っています。最初のコードは保存してあるので、なんとかはなるのですが、ひどいコードを書いていたものだとおもっています。更新したプログラムのどこが悪いのでしょうか。よろしくおねがいします。

[LouiS0616]

BEST_TEN *plst[*count];
ポインタ変数をcount数分しか確保していないのに、
for (i=0;i<10; i++) {
....printf("%d年 %d月 %d日 %d時 %d分 %d秒 \n"
........,plst[i]->tm_year+1900, plst[i]->tm_mon+1, plst[i]->tm_mday,
........plst[i]->tm_hour,plst[i]->tm_min,plst[i]->tm_sec);
....printf("得点（所要時間）は %.1f秒です。\n\n", plst[i]->score);
}
毎回plst[9]まで参照しているからじゃないですかね。

[LouiS0616]

エラーを一つ一つ潰せばいつか動くとは思いますが、ただそれだけです。
後から読み返したらもう理解できないでしょうし、機能追加なんて検討しようものなら地獄です。
全体の設計を見直した方がいいと思います。

[退会済みユーザー]

ありがとうございます。やってみます。

[退会済みユーザー]

plst[i]は整列した後の実行時間の順になっているので10個短い順にひろっているだけです。

[退会済みユーザー]

今日夜勤で寝ていないので、明日朝からプログラムをチェックする予定です。
ありがとうございます。

Answer 3

見やすい見やすくないは個人差があるので、この回答が全て正しいとは思いません。

以下、見づらいなと思った理由です。

不定な位置のコメント

右付きだったり上付きだったり。

統一されていないスタイル

• if の後の空白
• for の中の空白
• = の前後の空白

統一されていないエラー処理

エラー処理があったりなかったり。

明瞭でない関数名

関数名の意味が見ただけで分からない。

early return になってない

処理を中断しているのでもう return してよいのにしていない。

おまけ

気になった箇所だけ書き換えてみました。エラー処理を追加しているので処理の流れが変わってしまっていたらすみません。

見ただけで意味が伝わりにくい関数名について直そうと思いましたが、良く分からないので断念しました。

// ラックナンバーサーチ・トレーニング
// 過去の履歴とそれらの実行日時,実行時間
// 過去のbesttenの実行日時,実行時間
// 最近の10回の実行日時,実行時間
#include <stdio.h>
#include <time.h>
#include <float.h>
#include <ctype.h>
#include <stdlib.h>
#include <sys/types.h>
#include <unistd.h>
#include "getputch.h"

#define MAX_STAGE    3
#define swap(type, x, y)    do { type t = x; x = y; y = t; } while (0)
#define MAX_NUM         10

char dtfile[]  = "LACKNUM.DAT";
char dtfile2[] = "LACKNUM2.DAT";/* 最高記録用のファイル */

typedef struct {
  int tm_year;
  int tm_mon;
  int tm_mday;
  int tm_hour;
  int tm_min;
  int tm_sec;
  double best;
} BEST_TEN;

// --------------- 比較用の関数 cmp -------------------
static int
cmpptr( const void *p, const void *q ) {
  return (*(BEST_TEN**)p)->best - (*(BEST_TEN**)q)->best;
}
// ----------------------------------------------------

//1回実行したとき進むポイントの大きさを求める

static int
get_point_offset(fpos_t* offset) {
  FILE *fp;
  struct tm local;
  double best;

  fp = fopen(dtfile, "rb");
  if (fp == NULL) {
    perror("fopen");
    return -1;
  }

  if (fread(&local, sizeof(struct tm), 1, fp) < 0) {
    perror("fread");
    fclose(fp);
    return -1;
  }

  if (fread(&best, sizeof(double), 1, fp) < 0) {
    perror("fread");
    fclose(fp);
    return -1;
  }

  //ファイルポインタの位置を取得
  if (fgetpos(fp, offset) != 0) {   //ファイルfpの現在のファイル位置を取得して
    perror("fgetpos");
    fclose(fp);
    return -1;
  }

  fclose(fp);

  // offsetの示す場所に格納します。
  printf("ファイルポインタの位置offsetは「%d」です。\n", *offset);
  return 0;
}

//local のデータを新しい順にならべる。
static void
update_local_data(fpos_t ft,int k) {
  FILE *fp;        // これまでの履歴を指すポインタ
  FILE *fp2;        // これまでの最短所要時間を指すポインタ
  double best;    //これまでの履歴の所要時間
  double best2;   //これまでの最短所要時間のベストテンも保存しておく
  int i;

  struct tm local;//構造体変数 localの大きさ

  if ((fp = fopen(dtfile, "rb")) == NULL) {
    printf("ファイルを作成します。\n\n");
    best = DBL_MAX;  /* float.hに定義されている。double型で表現できる最大値を表すマクロDBL_MAX */
    return;
  }

  printf("最近の10回の点数とそれらの実行日時は\n\n");
  for (i = ft - k; i >= ft - 10 * k; i -= k) { //構造体変数 localの大きさoffsetで44
    //ファイルポインタを先頭からiまで移動
    if (fseek(fp, i, SEEK_SET) != 0) {
      perror("fseek");
      break;
    }

    if (fread(&local, sizeof(struct tm), 1, fp) < 0) {
      perror("fread");
      break;
    }
    printf("%d年 %d月 %d日 %d時 %d分 %d秒\n",
        local.tm_year + 1900, local.tm_mon + 1,
        local.tm_mday, local.tm_hour, local.tm_min, local.tm_sec);
    if (fread(&best, sizeof(double), 1, fp) < 0) {
      perror("fread");
      break;
    }
    printf("得点（所要時間）は%.1f秒\n\n", best);
  }
  fclose(fp);

  printf("enterキーで開始します。\n");
  fflush(stdin);
  getchar();
}

/*--- 過去のトレーニング情報を取得・表示して最高得点を返す ---*/
static double
get_data(int *count) {
  FILE *fp;        /* これまでの履歴を指すポインタ */
  FILE *fp2;        /* これまでの最短所要時間を指すポインタ */
  double best;            //これまでの履歴の所要時間
  double best2;           //これまでの最短所要時間
  int i,j=0;
  BEST_TEN best_ten[100]={0};
  BEST_TEN *plst[*count];
  struct tm local;
  double line[256];

  if ((fp = fopen(dtfile, "rb")) == NULL) {
    printf("ファイルを作成します。\n\n");
    best = DBL_MAX;  /* float.hに定義されている。double型で表現できる最大値を表すマクロDBL_MAX */
    return 0.0;
  }

  printf("\n過去の履歴\n-------------------------- \n");

  while((i = fread(&local, sizeof(struct tm), 1, fp)) > 0 ){
    printf("%d年 %d月 %d日 %d時 %d分 %d秒\n",
        local.tm_year + 1900, local.tm_mon + 1,
        local.tm_mday, local.tm_hour, local.tm_min, local.tm_sec);
    fread(&best, sizeof(double), 1, fp);
    printf("得点（所要時間）は%.1f秒\n\n", best);

    best_ten[j].tm_year = local.tm_year;
    best_ten[j].tm_mon = local.tm_mon;
    best_ten[j].tm_mday = local.tm_mday;
    best_ten[j].tm_hour = local.tm_hour;
    best_ten[j].tm_min = local.tm_min;
    best_ten[j].tm_sec = local.tm_sec;
    best_ten[j].best = best;
    j++;
    (*count)++;
  }
  printf("count1は%d\n\n", *count);

  for(i = 0; i < *count; i++)
    plst[i] = &best_ten[i];

  qsort(plst, *count, sizeof(BEST_TEN*), cmpptr);

  printf("\n過去のbestten\n-------------------------- \n");

  // 並べ替え後の内容を表示
  for (i = 0;i < 10; i++) {
    printf("%d年 %d月 %d日 %d時 %d分 %d秒 \n"
        ,plst[i]->tm_year+1900, plst[i]->tm_mon+1, plst[i]->tm_mday,
        plst[i]->tm_hour,plst[i]->tm_min,plst[i]->tm_sec);
    printf("得点（所要時間）は %.1f秒です。\n\n", plst[i]->best);
  }

  best = plst[0]->best;

  fflush(stdin);
  fclose(fp);

  printf("count2は%d\n\n", *count);
  return best;
}

/*--- 今回のトレーニング情報を書き込む ---*/
static int
put_data(double best, double best2) {
  FILE *fp;    //fpはfopenされたdtfileを指すポインタ
  FILE *fp2;  //fp2はfopenされたdtfile2を指すポインタ、最短時間が格納されている
  time_t t = time(NULL);
  struct tm *local = localtime(&t);

  time(&t);
  local = localtime(&t);
  if ((fp = fopen(dtfile, "ab")) == NULL) {
    printf("ファイルがあーりません");
    fflush(stdout);
    return -1;
  }

  if (fwrite(local, sizeof(struct tm), 1, fp) != 0) { //fpに書き込む
    perror("fwrite");
    fclose(fp);
    return -1;
  }

  if (fwrite(&best, sizeof(double), 1, fp) != 0) { //fpに書き込む
    perror("fwrite");
    fclose(fp);
    return -1;
  }

  fclose(fp);

  if (best > best2) {
    return 0;
  }

  if ((fp2 = fopen(dtfile2, "wb")) == NULL) {
    printf("ファイルがあーりません");
    fflush(stdout);
    return -1;
  }

  //point配列にbestをいれ、best2をベスト2にいれる。dtfile[0]にbest1,dtfile[1]にbest2.....dtfile[9]にbest10
  if (fwrite(local, sizeof(struct tm), 1, fp2) != 0) {
    perror("fwrite");
    fclose(fp);
    return -1;
  }

  if (fwrite(&best2, sizeof(double), 1, fp2) != 0) {
    perror("fwrite");
    fclose(fp);
    return -1;
  }

  fclose(fp2);
  return 0;
}

/*--- トレーニングを実行して得点（所要時間）を返す ---*/
static double
do_training(void) {
  int i, j, stage;
  int dgt[9] = {1, 2, 3, 4, 5, 6, 7, 8, 9};
  int a[8];
  double jikan;       // 時間
  clock_t start, end; // 開始時刻・終了時刻

  printf("\n\n欠けている数字を入力してください。\n");
  printf("スペースキーで開始します。\n");
  while (getch() != ' ')
    ;
  start = time(NULL);

  for (stage = 0; stage < MAX_STAGE; stage++) {
    int x = rand() % 9; /* 0～8の乱数を発生 ,1から9までのうち8文字を表示するための数字 */
    int    no;          /* 読み込んだ値 ,キーボードから打ち込んだ数字 */

    i = j = 0;
    /* dgt[x]を飛ばしてコピー */
    while (i < 9) {
      if (i != x)
        /* dgt[x]は1から9までの数字が入っている配列 */
        a[j++] = dgt[i];
      i++;
    }

    /* 配列aをシャッフル、配列の添字は0から7までの8個である */
    /* 配列aの添字は0から7までの8個である。jは1から7まで変化する */
    /* jは1から7まで変化する。iとjの添字でシャッフルする。 */
    for (i = 7; i > 0; i--) {
      int j = rand() % (i + 1);
      if (i != j)
        swap(int, a[i], a[j]);
    }

    printf("%d回目：", stage+1);

    /* 全要素を表示 */
    for (i = 0; i < 8; i++) {
      printf("%d ", a[i]);
    }
    printf("：");
    fflush(stdout);

    do {
      no = getch();
      if (isprint(no)) {          /* 表示可能であれば */
        putch(no);                /* 押されたキーを表示 */
        if (no != dgt[x] + '0')   /* 正解でなければ */
          putch('\b');            /* カーソルを一つ戻す */
        else
          printf("\n");           /* 改行 */
      }
    } while (no != dgt[x] + '0');
  }
  end = time(NULL);

  jikan = (double)difftime(end, start);

  printf("%.1f秒かかりました。\n", jikan);

  if (jikan > 25.0)
    printf("鈍すぎます。\n");
  else if (jikan > 20.0)
    printf("少し鈍いですね。\n");
  else if (jikan > 17.0)
    printf("まあまあですね。\n");
  else
    printf("素早いですね。\n");

  return jikan;
}

int
main(void) {
  int retry, count = 0;  // もう一度？
  double score;          // 今回の所要時間
  double best;           // 最短所要時間
  double jikan;          // 時間
  FILE *fp;
  fpos_t ft;
  fpos_t offset;
  struct tm local;
  int k;
  int i, j, x, stage;

  // get_data()で前回までの最短所要時間を
  best = get_data(&count);

  // ファイルから読み込んでbestに代入する。
  printf("bestは%.1f\n\n", best);
  printf("count3は%d\n\n", count);

  fp = fopen(dtfile, "rb");
  if (fp == NULL) {
    perror("fopen");
    return -1;
  }

  if (get_point_offset(&offset) != 0) {
    return -1;
  }

  k = offset;
  //ファイルポインタの位置を取得
  printf("mainのget_point_offset(&offset)後のkの値は「%d」です。\n\n", k);

  //ファイルポインタを末尾まで移動
  if (fseek(fp, 0, SEEK_END) != 0) {
    perror("fseek");
    return -1;
  }

  //ファイルポインタの位置を取得
  //ファイルfpの現在のファイル位置を取得して
  if (fgetpos(fp, &ft) != 0) {
    perror("fgetpos");
    fclose(fp);
    return -1;
  }

  // &ftの示す場所に格納します。
  update_local_data(ft, k);

  if (fgetpos(fp, &ft) != 0) {
    perror("fgetpos");
    return -1;
  }
  fclose(fp);

  //best = get_data(ft);
  // get_data()で前回までの最短所要時間を
  //ファイルから読み込んでbestに代入する。

  //ライブラリの初期処理235p、initscr(),cbreak(),noecho(),refresh()
  init_getputch();

  // 乱数の種を初期化
  srand(time(NULL));
  do {
    // トレーニング(do_training)で実行、返却された所要時間(jika)を
    // scoreに代入する。
    score = do_training();
    if (score < best) {
      printf("最短所要時間を更新しました!!\n");
      /* 最高得点更新 */
      best = score;
    }

    printf("もう一度しますか … (0)いいえ (1)はい：");
    retry = 0;
    scanf("%d", &retry);
  } while (retry == 1);

  /* 今回の日付・時刻・得点を書き込む */
  put_data(score,best);
  fflush(stdout);

  //ライブラリの初期処理235p,endwin()
  term_getputch();

  return 0;
}

Comments

[退会済みユーザー]

mattnさんのコードを参考に自分なりに修正したのですが、最後までいかないでとまってしまいます。
どこがわるいのでしょうか？途中までの表示は正常に表示されています。debugをやってみたのですが
debugもうまくできないので、よろしくおねがいします。