質問をすることでしか得られない、回答やアドバイスがある。

15分調べてもわからないことは、質問しよう!

ただいまの
回答率

90.38%

  • C

    4821questions

    C言語は、1972年にAT&Tベル研究所の、デニス・リッチーが主体となって作成したプログラミング言語です。 B言語の後継言語として開発されたことからC言語と命名。そのため、表記法などはB言語やALGOLに近いとされています。 Cの拡張版であるC++言語とともに、現在世界中でもっとも普及されているプログラミング言語です。

  • C++

    4653questions

    C++はC言語をもとにしてつくられた最もよく使われるマルチパラダイムプログラミング言語の1つです。オブジェクト指向、ジェネリック、命令型など広く対応しており、多目的に使用されています。

  • Visual Studio

    2503questions

    Microsoft Visual StudioはMicrosoftによる統合開発環境(IDE)です。多種多様なプログラミング言語に対応しています。

自分の解答の穴を見つけて頂けると助かります。

解決済

回答 5

投稿 編集

  • 評価
  • クリップ 0
  • VIEW 572

現在、書籍「苦しんで覚えるc言語」でc言語を勉強しているプログラミング入門者です。
次のような練習問題に取り組んでいるのですが、自分なりに著者の解答を書き換えてみました。
他に著者の解答に付け足すべき箇所などありましたら、教えて頂けると幸いです。

前提

情報を入力する「InputPeople関数」内において、正しく入力されたか確認を取るために、著者の解答を一部書き換えました。

実現したいこと

より安全で正確な(穴のない)プログラムが書けたらと思っています。

問題

「3人分の、名前、年齢、性別、を入力して表示するプログラムを作りなさい。
ただし、データは構造体で記憶することとし、
また、データの入力と表示はそれぞれ専用の関数を作って行うこととする。」

著者の解答

#include<stdio.h>
#include<string.h>
typedef struct
{
    char name[256];
    int age;
    int sex;
}People;

void InputPeople(People *data);
void ShowPeople(People data);

int main(void)
{
    People data[3];
    int i;

    for (i = 0; i < 3; i++)
    {
        InputPeople(&data[i]);
    }
    for ( i = 0; i < 3; i++)
    {
        ShowPeople(data[i]);
    }
    return 0;
}


void InputPeople(People *data)
{
    printf("名前:");
    scanf_s("%s", &data->name,256);
    printf("年齢:");
    scanf_s("%d", &data->age);
    printf("性別(1-男性、2-女性):");
    scanf("%d", &data->sex);
    printf("\n");
}


void ShowPeople(People data)
{
    char sex[16];

    printf("名前:%s\n",data.name);
    printf("年齢:%d\n",data.age);


    if (data.sex == 1)
    {
        strcpy_s(sex, 16,"男性");
    }
    else
    {
        strcpy_s(sex, 16,"女性");
    }
    printf("性別:%s\n",sex);
    printf("\n");
}

自分で一部書き換えたプログラム

#include<stdio.h>
#include<string.h>
typedef struct
{
    unsigned char name[256];
    unsigned int age;
    unsigned int sex;
}People;

void InputPeople(People *data);
void ShowPeople(People data);

int main(void)
{
    People data[3];
    int i;

    for (i = 0; i < 3; i++)
    {
        InputPeople(&data[i]);
    }

    for (i = 0; i < 3; i++)
    {
        ShowPeople(data[i]);
    }
    return 0;
}

void InputPeople(People *data)
{
    int i,j, k;
    char sex[16];


    do
    {
        printf("名前を入力してください:");
        scanf_s("%s", data->name,256);
        printf("%sでよろしいですか??(「合ってます」:「1」を入力、「間違いました」:「1以外」を入力)\n", data->name);
        scanf_s("%d", &i);
    } while (i != 1);

    do
    {
        printf("年齢を入力してください:");
        scanf_s("%d", &data->age);
        printf("%dでよろしいですか??(「合ってます」:「1」を入力、「間違いました」:「1以外」を入力)\n", data->age);
        scanf_s("%d", &j);
    } while (j != 1);

    do
    {
        printf("性別を入力してください(「男性」:「1」を入力、「女性」:「2」を入力):");
        scanf_s("%d", &data->sex);
        if (data->sex == 1)
        {
            strcpy_s(sex, 16, "男性");
        }
        else if(data->sex == 2)
        {
            strcpy_s(sex, 16, "女性");
        }
        else
        {
            continue;
        }
        printf("%sでよろしいですか??(「合ってます」:「1」を入力、「間違いました」:「1以外」を入力)\n", sex);
        scanf_s("%d", &k);
    } while (k != 1);

    printf("\n");
}


void ShowPeople(People data)
{
    char sex[16];

    printf("名前:%s\n", data.name);
    printf("年齢:%d\n", data.age);
    if (data.sex == 1)
    {
        strcpy_s(sex, 16, "男性");
    }
    else
    {
        strcpy_s(sex, 16, "女性");
    }
    printf("性別:%s\n", sex);
    printf("\n");
}


「自分で一部書き換えたプログラム」に穴や間違いがありましたら、教えて頂けると助かります。

(本記事は、苦しんで覚えるc言語から一部引用しています。)

  • 気になる質問をクリップする

    クリップした質問は、後からいつでもマイページで確認できます。

    またクリップした質問に回答があった際、通知やメールを受け取ることができます。

    クリップを取り消します

  • 良い質問の評価を上げる

    以下のような質問は評価を上げましょう

    • 質問内容が明確
    • 自分も答えを知りたい
    • 質問者以外のユーザにも役立つ

    評価が高い質問は、TOPページの「注目」タブのフィードに表示されやすくなります。

    質問の評価を上げたことを取り消します

  • 評価を下げられる数の上限に達しました

    評価を下げることができません

    • 1日5回まで評価を下げられます
    • 1日に1ユーザに対して2回まで評価を下げられます

    質問の評価を下げる

    teratailでは下記のような質問を「具体的に困っていることがない質問」、「サイトポリシーに違反する質問」と定義し、推奨していません。

    • プログラミングに関係のない質問
    • やってほしいことだけを記載した丸投げの質問
    • 問題・課題が含まれていない質問
    • 意図的に内容が抹消された質問
    • 広告と受け取られるような投稿

    評価が下がると、TOPページの「アクティブ」「注目」タブのフィードに表示されにくくなります。

    質問の評価を下げたことを取り消します

    この機能は開放されていません

    評価を下げる条件を満たしてません

    評価を下げる理由を選択してください

    詳細な説明はこちら

    上記に当てはまらず、質問内容が明確になっていない質問には「情報の追加・修正依頼」機能からコメントをしてください。

    質問の評価を下げる機能の利用条件

    この機能を利用するためには、以下の事項を行う必要があります。

質問への追記・修正、ベストアンサー選択の依頼

  • LouiS0616

    2019/01/05 20:50

    ・コードブロックを利用してください。
    コードが読みづらいです。次のgif画像に従って質問を編集して下さい。
    https://teratail.storage.googleapis.com/uploads/contributed_images/4c6e48a3bd0707d89f61b901fd1f8915.gif

    ・観点を追記してください。
    どのような経緯で著者の回答を改造したのかいまいちわかりません。
    改変した部分とその理由を追記してください。理由は詳しい方が良いですが、必ずしも厳密なものでなくても構いません。

    キャンセル

  • bochan2

    2019/01/05 20:57

    質問いただきありがとうございます!
    自分で行った確認作業(デバッグ等)について追記頂けると回答の役に立つと思います

    キャンセル

回答 5

checkベストアンサー

0

入力チェック用のフラグ変数にi,j,kといった名称を使うのはお勧めしません。

nameCheckFlag, sexCheckFlagなど意味のある変数名にした方がよいでしょう。
変数や関数の名前ってソースの可読性に大きな影響がありますよ。
リーダブルコードなどの書籍を読むと勉強になると思います。

正しく入力されたか確認を取るために、

自分がやるとすると1項目ずつ確認を入力させるというやり方はとらないと思います。
プログラムを使う側の立場に立ってみるとイライラすると思いませんか?
すべての項目を入力した後にOKかどうか再入力する項目の確認を促すようにすると思います。

投稿

  • 回答の評価を上げる

    以下のような回答は評価を上げましょう

    • 正しい回答
    • わかりやすい回答
    • ためになる回答

    評価が高い回答ほどページの上位に表示されます。

  • 回答の評価を下げる

    下記のような回答は推奨されていません。

    • 間違っている回答
    • 質問の回答になっていない投稿
    • スパムや攻撃的な表現を用いた投稿

    評価を下げる際はその理由を明確に伝え、適切な回答に修正してもらいましょう。

  • 2019/01/14 11:04

    貴重なアドバイスを有難うございます。

    キャンセル

0

入力の検証をしたいようですが、そもそもscanf系関数で数値読み込みすること自体が問題です。

C言語で安全に標準入力から数値を取得

を見てください。

あとagesexint型なのが個人的にはどうかと思うのでenumとかunsignedな何かに書き換えたいt頃です。


暇ができたので全面書き直ししました。他にもいろいろ思うところがあったので。

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <limits.h>
#include <errno.h>
#include <stdbool.h>
#include <ctype.h>
#include <float.h>
#include <assert.h>
#ifndef static_assert
#   define NO_STDC_STATIC_ASSERT
#   define static_assert(...)
#endif
#if defined(_MSC_VER) || defined(__cplusplus)
#   define restrict
#endif
/**
 * @brief 文字列が文字を持っているか調べます。
 * @param str 対象文字列へのポインタ
 * @return false: nullptrか空白文字のみの文字列 true:それ以外
 */
static inline bool str_has_char(const char *str)
{
    if (NULL == str) return false;
    bool ret = false;
    for (; !ret && *str != '\0'; str++) ret = (*str != ' ');
    return ret;
}
/**
 * @brief 文字列が文字を持っているか調べます。
 * @param io 書き換えるbool型変数へのポインタ、呼び出し後はポインタが指す変数にnew_valueが代入される
 * @param new_value 新しい値
 * @return ioが指すbool変数がもともと持っていた値
 */
static inline bool exchange_bool(bool* restrict const io, const bool new_value)
{
    const bool tmp = *io;
    *io = new_value;
    return tmp;
}
/**
 * @brief fgetsで失敗したときにストリームをクリアしてループする関数
 * @param s ストリームから読み取った文字列を格納するための領域へのポインタ
 * @param buf_size ストリームから読み取った文字列を格納するための領域の大きさ
 * @param stream FILE構造体へのポインタかstdin
 * @param message_on_error エラー時に表示してループする
 * @return 成功時は0, new line at the end of fileのときは-1
 */
static inline int fgets_wrap(char* restrict const s, size_t buf_size, FILE* restrict const stream, const char* restrict message_on_error)
{
    size_t i = 0;
    for (bool first_flg = true; i < 100 && NULL == fgets(s, buf_size, stream); ++i) {
        if (feof(stdin)) return -1;
        if (!exchange_bool(&first_flg, false)) puts((message_on_error) ? message_on_error : "再入力してください");
    }
    if (100u == i) exit(1);//無限ループ防止
    if (feof(stdin)) return 0;
    //改行文字が入力を受けた配列にない場合、入力ストリームにごみがある
    const size_t len = strlen(s);
    //短すぎる入力
    if (0 == len || (1 == len && '\n' == s[0])) return 1;
    //長過ぎる入力
    if ('\n' != s[len - 1]) {
        //入力ストリームを掃除
        while (fgetc(stream) != '\n');
        return 2;
    }
    return 0;
}

/**
 * @brief 標準入力から文字列の入力を受ける
 * @param s ストリームから読み取った文字列を格納するための領域へのポインタ
 * @param buf_size ストリームから読み取った文字列を格納するための領域の大きさ
 * @param message 入力を受ける前にputsに渡す文字列。表示しない場合はnullptrか空白文字のみで構成された文字列へのポインタを渡す
 * @param message_on_error エラー時に表示してループする
 */
static inline void input_str(char* restrict const s, size_t buf_size, const char* message, const char* restrict message_on_error)
{
    if (str_has_char(message)) puts(message);
    size_t i = 0;
    for (; i < 100u; ++i) {
        //長過ぎる入力以降の無限ループ防止にerrnoをクリアする
        errno = 0;
        switch (fgets_wrap(s, buf_size, stdin, message_on_error)) {
        case -1: return;//EOF
        case 1://短すぎる入力
        case 2://長過ぎる入力
            continue;
        default: goto after_loop;
        }
    }
    if (100u == i) exit(1);//無限ループ防止
after_loop:
    {
        char* const lf = strchr(s, '\n');
        if(NULL != lf) lf[0] = '\0';
    }
}

/**
 * @brief 標準入力から入力を受け、unsigned int型に変換する
 * @details fgetsしてstrtodしている。max, minの条件に合わないかエラー時はループ
 * @details errnoの値を書き換える
 * @param message 入力を受ける前にputsに渡す文字列。表示しない場合はnullptrか空白文字のみで構成された文字列へのポインタを渡す
 * @param message_on_error エラー時に表示してループする
 * @param max 入力値を制限する。最大値を指定
 * @param min 入力値を制限する。最小値を指定
 * @return 入力した数字、EOFのときは0
 */
static inline unsigned int input_uint(const char* message, const char* restrict message_on_error, const unsigned int max, const unsigned int min)
{
    if (str_has_char(message)) puts(message);
    char s[30];
    static_assert(sizeof(unsigned int) < 8, "err");
    unsigned long t = 0;
    size_t i = 0;
    for (char* endptr = s; ((0 == t && endptr == s) || 0 != errno || t < min || max < t) && i < 100u; ++i) {
        //長過ぎる入力以降の無限ループ防止にerrnoをクリアする
        errno = 0;
        switch (fgets_wrap(s, sizeof(s), stdin, message_on_error)) {
        case -1: return 0;//EOF
        case 1://短すぎる入力
        case 2://長過ぎる入力
            endptr = s;//ループ制御フラグとして流用
            continue;
        default: break;
        }
        t = strtoul(s, &endptr, 10);
    }
    if (100 == i) exit(1);//無限ループ防止
    return ((unsigned int)(t));
}
#ifdef NO_STDC_STATIC_ASSERT
#   undef static_assert
#   undef NO_STDC_STATIC_ASSERT
#endif

#ifndef _countof
#define _countof(arr) (sizeof(arr) / sizeof(*arr))
#endif

typedef struct
{
    char name[256];
    unsigned int age;
    unsigned int sex;
}People;

void InputPeople(People *data)
{
    do {
        input_str(data->name, _countof(data->name), "名前を入力してください", "不正な長さの入力です、もう一度入力してください");
        printf("%sでよろしいですか??(「合ってます」:「1」を入力、「間違いました」:「1以外」を入力)\n", data->name);
    } while (!feof(stdin) && 1 != input_uint(NULL, NULL, UINT_MAX, 0));
    if (feof(stdin)) exit(1);
    do {
        data->age = input_uint("年齢を入力してください", NULL, UINT_MAX, 0);
        printf("%dでよろしいですか??(「合ってます」:「1」を入力、「間違いました」:「1以外」を入力)\n", data->age);
    } while (!feof(stdin) && 1 != input_uint(NULL, NULL, UINT_MAX, 0));
    if (feof(stdin)) exit(1);
    do {
        data->sex = input_uint("性別を入力してください(「男性」:「1」を入力、「女性」:「2」を入力)", NULL, 2, 1);
        printf("%sでよろしいですか??(「合ってます」:「1」を入力、「間違いました」:「1以外」を入力)\n", (1 == data->sex) ? "男性" : "女性");
    } while (!feof(stdin) && 1 != input_uint(NULL, NULL, UINT_MAX, 0));
    if (feof(stdin)) exit(1);
    putchar('\n');
}
void ShowPeople(const People* data)
{
    printf(
        "名前:%s\n"
        "年齢:%d\n"
        "性別:%s\n",
        data->name, data->age, (data->sex == 1) ? "男性" : "女性"
    );
    putchar('\n');
}

int main(void)
{
    People data[3];
    for (size_t i = 0; i < 3; i++) {
        InputPeople(&data[i]);
    }
    for (size_t i = 0; i < 3; i++) {
        ShowPeople(&data[i]);
    }
    return 0;
}

https://wandbox.org/permlink/NuxCyJqbMdnrsPCc

追記:

Qiitaに記事書きました、上記コードに誤りがあった場合はQiitaのほうのみ修正することにします。
「苦しんで覚えるc言語」のとあるコードをもっと安全に、より良く

投稿

編集

  • 回答の評価を上げる

    以下のような回答は評価を上げましょう

    • 正しい回答
    • わかりやすい回答
    • ためになる回答

    評価が高い回答ほどページの上位に表示されます。

  • 回答の評価を下げる

    下記のような回答は推奨されていません。

    • 間違っている回答
    • 質問の回答になっていない投稿
    • スパムや攻撃的な表現を用いた投稿

    評価を下げる際はその理由を明確に伝え、適切な回答に修正してもらいましょう。

  • 2019/01/07 19:25

    回答有難うございます。大変参考になりました。

    キャンセル

  • 2019/01/14 12:06

    有難うございます。
    自習に使用させて頂きます。

    キャンセル

0

全般に冗長にしただけのイメージです。
元のコードもどうかなって気がしますが、、、気になったところを

  • 入力の確認

while (i == 2);

3 とか入れたら、どうなるでしょうか? 多分、そのまま、OKですね。 良いですか?
(他も同様)

  • 性別の判定

if (data->sex == 1) {

個々も同様です。 1,2 以外はどうしましょう?

  • 文字列コピー

strcpy_s(sex, 16, "男性");

sex の文字列は表示でしか使っていません。
char *sex;  として、sex = "男性"; で十分じゃないですか?

気が付いたところで。

投稿

  • 回答の評価を上げる

    以下のような回答は評価を上げましょう

    • 正しい回答
    • わかりやすい回答
    • ためになる回答

    評価が高い回答ほどページの上位に表示されます。

  • 回答の評価を下げる

    下記のような回答は推奨されていません。

    • 間違っている回答
    • 質問の回答になっていない投稿
    • スパムや攻撃的な表現を用いた投稿

    評価を下げる際はその理由を明確に伝え、適切な回答に修正してもらいましょう。

  • 2019/01/10 18:00

    ご返信有難うございます。
    勉強になりました。

    キャンセル

0

手軽にチェックする方法にはつぎのものがあります。

  • cppcheck などのフリーの静的解析を使用する
  • コンパイラの警告オプションを変更する

投稿

  • 回答の評価を上げる

    以下のような回答は評価を上げましょう

    • 正しい回答
    • わかりやすい回答
    • ためになる回答

    評価が高い回答ほどページの上位に表示されます。

  • 回答の評価を下げる

    下記のような回答は推奨されていません。

    • 間違っている回答
    • 質問の回答になっていない投稿
    • スパムや攻撃的な表現を用いた投稿

    評価を下げる際はその理由を明確に伝え、適切な回答に修正してもらいましょう。

0

i, j, k はマズいね。その名前が意味を持たないから。
あとで読み返すことを考えた方がいい。体を表す名を与えよ。

※ ~CheckFlag は感心しない。この文脈では validName, validGender とかそんなカンジ。

投稿

編集

  • 回答の評価を上げる

    以下のような回答は評価を上げましょう

    • 正しい回答
    • わかりやすい回答
    • ためになる回答

    評価が高い回答ほどページの上位に表示されます。

  • 回答の評価を下げる

    下記のような回答は推奨されていません。

    • 間違っている回答
    • 質問の回答になっていない投稿
    • スパムや攻撃的な表現を用いた投稿

    評価を下げる際はその理由を明確に伝え、適切な回答に修正してもらいましょう。

  • 2019/01/14 11:03

    大変貴重なアドバイスをありがとうございます。
    勉強になりました。

    キャンセル

15分調べてもわからないことは、teratailで質問しよう!

  • ただいまの回答率 90.38%
  • 質問をまとめることで、思考を整理して素早く解決
  • テンプレート機能で、簡単に質問をまとめられる

同じタグがついた質問を見る

  • C

    4821questions

    C言語は、1972年にAT&Tベル研究所の、デニス・リッチーが主体となって作成したプログラミング言語です。 B言語の後継言語として開発されたことからC言語と命名。そのため、表記法などはB言語やALGOLに近いとされています。 Cの拡張版であるC++言語とともに、現在世界中でもっとも普及されているプログラミング言語です。

  • C++

    4653questions

    C++はC言語をもとにしてつくられた最もよく使われるマルチパラダイムプログラミング言語の1つです。オブジェクト指向、ジェネリック、命令型など広く対応しており、多目的に使用されています。

  • Visual Studio

    2503questions

    Microsoft Visual StudioはMicrosoftによる統合開発環境(IDE)です。多種多様なプログラミング言語に対応しています。