プログラムの関数化

質問

皆様に助けていただき、なんとかコーディング出来たのですが、関数化したいと思います。アドバイスお願いします。

int overflow_check1();
int overflow_check2();
int overflow_check3();
int overflow_check4();
int lop; // 左のオペランドに入力する値
int rop; // 右のオペランドに入力する値
int result; // 計算結果
int mod; // 除算の余り
int max = INT_MAX; // 表現可能な最大値
int min = INT_MIN; // 表現可能な最小値
char *check; // 変換不能な文字を格納
char op; // 入力する演算子
int main ( int argc, char *argv[] )
{
/* 引数の個数チェック */
if ( argc != 4 ) {
printf ( "usage : %s lop op rop\n", argv[0] );
exit ( 0 );
}
/* 引数を取得して整数型に変換 */
lop = atoi ( argv[1] );
rop = atoi ( argv[3] );
/* 引数の取得 */
op = ( argv[2][0] );
/* startが整数であるか文字であるかのチェック */
lop = strtol ( argv[1], &check, 10 );
if ( errno != ERANGE ) {
if ( *check != '\0' ) {
printf ( "error : 整数を入力して下さい。\n" );
exit ( -1 );
}
/* startに範囲を超えた値を入力した場合 */
} else {
printf( "error : 整数型の範囲を超えました。\n" );
exit( -1 );
}
/* endが整数であるか文字であるかのチェック */
rop = strtol ( argv[3], &check, 10 );
if ( errno != ERANGE ) {
if ( *check != '\0' ) {
printf ( "error : 整数を入力して下さい。\n" );
exit ( -1 );
}
/* endに範囲を超えた値を入力した場合 */
} else {
printf( "error : 整数型の範囲を超えました。\n" );
exit( -1 );
}
/* argv[2]が2文字以上の場合 */
if ( strlen ( argv[2] ) >= 2 ) {
printf ( "error : 演算子を正しく入力して下さい。\n" );
exit ( -1 );
}
/* argv[2]に入力された演算子が'+'の場合 */
switch ( op ) {
case '+':
overflow_check1();
result = lop + rop;
break;
/* argv[2]に入力された演算子が'-'の場合 */
case '-' :
overflow_check2();
result = lop - rop;
break;
/* argv[2]に入力された演算子が'*'の場合 */
case '*':
overflow_check3();
result = lop * rop;
break;
/* argv[2]に入力された演算子が'/'の場合 */
case '/' :
/* 0による除算 */
if ( rop == 0 ) {
printf ( "error : 0による除算は禁止です。\n" );
exit ( -1 );
}
overflow_check4();
result = lop % rop; // 剰余を求める
break;
default:
printf ( "error : '+' '-' '*' '/'のいずれかの演算子を入力して下さい。\n" );
exit ( -1 );
}
/* 割り算の結果として余りが出なかった場合 */
if ( result == 0 ) {
result = lop / rop;
printf ( "%d %c %d = %d", lop, op, rop, result );
exit ( 0 );
/* 割り算の結果として余りが出た場合 */
} else {
result = lop / rop;
mod = lop % rop;
printf ( "%d %c %d = %d余り%d", lop, op, rop, result, mod );
exit ( 0 );
}
/* 数値1 演算子 数値2 = 演算結果の形式で出力する */
printf ( "%d %c %d = %d", lop, op, rop, result );
exit ( 0 );
}
int overflow_check1 ()
{
/* オーバーフローのチェック */
if ( lop > 0 && rop > 0 ) {
if ( lop > max - rop ) {
printf ( "error : オーバーフローが起こります。\n" );
exit ( 0 );
}
}
/* オーバーフローのチェック */
if ( lop < 0 && rop < 0 ) {
if ( lop < min - rop ) {
printf ( "error : オーバーフローが起こります。\n" );
exit ( 0 );
}
}
return 0;
}
int overflow_check2 ()
{
/* オーバーフローのチェック */
if ( lop > 0 && rop < 0 ) {
if ( lop > max + rop ) {
printf ( "error : オーバーフローが起こります。\n" );
exit ( 0 );
}
}
/* オーバーフローのチェック */
if ( lop < 0 && rop > 0 ) {
if ( lop < min + rop ) {
printf ( "error : オーバーフローが起こります。\n" );
exit ( 0 );
}
}
return 0;
}
int overflow_check3 ()
{
/* オーバーフローのチェック */
if ( lop > max / rop ) {
printf ( "error : オーバーフローが起こります。\n" );
exit ( -1 );
}
/* オーバーフローのチェック */
if ( rop < max / lop ) {
printf ( "error : オーバーフローが起こります。\n" );
exit ( 0 );
}
return 0;
}
int overflow_check4 ()
{
/* オーバーフローのチェック */
if ( ( lop == min ) && ( rop == -1 ) ) {
printf ( "error : オーバーフローが起こります。\n" );
exit ( -1 );
}
return 0;
}

回答No.2

ibe_tattsu · ベストアンサー

お返事ありがとうございました。

先ほどは、プログラムをよく検証せずに方法のみで返事をさせていただいたのですが、
よくよく見ると、このプログラムは正常に動くのでしょうか？

実際に引数を入力して動かしてみると、
#a.out 3 - 1
3 - 1 = 3 0
となりました。正しい結果じゃないですね＾＾；

上手く動作しないということ以外でなければ、プログラムを提示するときは、動作を一応確認しておいてください。
今回の質問者さんの目的は関数化ですし。

それとこの質問から見始めた方もいると思うので、プログラムの仕様なども記載しておくべきと思います。

＞ちなみに少しだけ関数化させてみたのですが、あまり意味のない関数化だと思い、全く自信がありません。
経験がなければ、自信が無いのは普通です。経験と共にきれいなプログラムが書けるようになってきますし、
最初はメイン関数のみになるというのも普通のことです。
今回、関数化されているところは私は適切だと思いますし、
まずはできるだけ自信を持って、自分なりにやってみてください。

お礼

一応オーバーフローも考慮して、ちゃんと動くようになりましたよ。
開発環境はvc2005です。ファイル名 オペランド 演算子 オペランド
と入力して実行すればちゃんと動きますよ！

<<今回、関数化されている部分は適切
ありごとうございます。少しずつ関数になれていきたいと思います。

補足

失礼しました。確認したところ確かにバグがありました。

case '/' :

/* 0による除算 */
if ( rop == 0 ) {
printf ( "error : 0による除算は禁止です。\n" );
exit ( -1 );
}
overflow_check4();
result = lop % rop; // 剰余を求める

/* 割り算の結果として余りが出なかった場合 */
if ( result == 0 ) {
result = lop / rop;
printf ( "%d %c %d = %d", lop, op, rop, result );
exit ( 0 );

/* 割り算の結果として余りが出た場合 */
} else {
result = lop / rop;
mod = lop % rop;
printf ( "%d %c %d = %d余り%d", lop, op, rop, result, mod );
exit ( 0 );
}
break;

default:
printf ( "error : '+' '-' '*' '/'のいずれかの演算子を入力して下さい。\n" );
exit ( 0 );
}

/* 数値1 演算子 数値2 = 演算結果の形式で出力する */
printf ( "%d %c %d = %d", lop, op, rop, result );
exit ( 0 );
}

ちなみに掛け算のオーバーフローのコードが間違っている気が
するのですが、これで正しいのでしょうか？

回答No.7

MrBan

> ありがとうございました。一応こんな感じでやってみたのですが
> 除算の部分で、result = ごとに条件が違うため、どうすればよいのか
> 困っています。

条件の違いとは？単純に「余り」の有無なのでは？
であれば、私の元コードでも条件で出力は変わってます。

(構造体のようなものはあえて使わずポインタ引数で処理してますが)

ちなみに、私のコードでは「余り」は「mod」に格納されresultには「余り」が入りません。
mod = lop % rop;
result = lop / rop;
になるようにコードが書かれており、同じresultを使いまわしたりしてません。
(変数の使いまわしは理解しにくいコードになりがちですので)

そして、「余り」を表示するか否かは「余りがあるかないか」で決まるのであり、
結果として「余りがなければ表示しない」のであれば、
加減乗賛の際にはmodを0(加減乗には余りがない)にしておけば、
割り算とか掛け算とかを区別する必要はなくなります。
(私のコードではこういう処理を行っています)

回答No.6

MrBan

/** 四則演算 */
static int
calc(int lop, char op, int rop, int* mod)
{
　int　result　=0;
　assert(mod != NULL);
　*mod　= 0;　/* 除算以外は0を保証(print_resultで"余り"をはじく) */

　switch(op)
　{
　case '+':　result　= add(lop, rop);　　break;
　case '-':　result　= sub(lop, rop);　　break;
　case '*':　result　= mul(lop, rop);　　break;
　case '/':　result　= div(lop, rop, mod);　break;
　default :　check_ok_or_exit(ALWAYS_ERROR, -1, UNKNOWN_OP_MSG);　return -1;　/* no return */
　}
　return result;　// ←抜けてました。
}

お礼

ありがとうございました。一応こんな感じでやってみたのですが
除算の部分で、result = ごとに条件が違うため、どうすればよいのか
困っています。

/* 加算 */
int add ( int lop, int rop )
{
if ( ( lop > 0 && rop > 0 ) && ( lop > max - rop ) ) {
printf ( "error : オーバーフローが起こります。\n" );
exit ( 0 );

} else if ( ( lop < 0 && rop < 0 ) && ( lop < min - rop ) ) {
printf ( "error : オーバーフローが起こります。\n" );
exit ( 0 );
}
return lop + rop;
}

/* 減算 */
int sub ( int lop, int rop )
{
if ( ( lop > 0 && rop < 0 ) && ( lop > max + rop ) ) {
printf ( "error : オーバーフローが起こります。\n" );
exit ( 0 );

} else if ( ( lop < 0 && rop > 0 ) && ( lop < min + rop ) ) {
printf ( "error : オーバーフローが起こります。\n" );
exit ( 0 );
}
return lop - rop;
}

/* 乗算 */
int mul ( int lop, int rop )
{
if ( ( lop > 0 && rop > 0 ) && ( lop > max / rop ) ) {
printf ( "error : オーバーフローが起こります。\n" );
exit ( 0 );
}

if ( ( ( lop == min ) && ( rop == -1 ) ) || ( ( lop == -1 ) && ( rop == min ) ) ) {
printf ( "error : オーバーフローが起こります。\n" );
exit ( 0 );
}
return lop * rop;
}

回答No.5

MrBan

Cの場合、エラーで一度戻すのが一般的なので、
関数内部で直接終了、というのが違和感がありますけど、
check_ok_or_exitは、Cでいえばassertに近い。
(ここでは条件が逆ですが合わせてもいいですし)

# C++とかだと例外処理ってのがあるので、まだ理解しやすいでしょうけど。

関数分割ってのは突き詰めると、「一行コメントで内容が表せる」ところまでいくのですが、
この例で言えば、オーバフローを個別に分けるよりは、
計算処理とセットにしておいた方がまだ分かりやすいように思います。
# ザックリ直すとこんな感じ？少なくともoverflow_check1～4とかいう"名前"はよくないです。(そして粒度も細かすぎると思います)

/* エラーが起きるとexitで抜けてしまう設計 */
#include <stdarg.h>
#include <string.h>
#include <stdlib.h>
#include <stdio.h>
#include <assert.h>
#include <limits.h>

/* $$$ プロトタイプ宣言省略 $$$ */

#define ALWAYS_ERROR　1　/* 常に失敗 */

/* エラーメッセージ */
static const char　OVERFLOW_MSG[]　　　= "error : オーバーフローが起こります。\n";
static const char　ZERO_DIV_MSG[]　　　= "error : 0による除算は禁止です。\n";
static const char　OUT_OF_RANGE_MSG[]　= "error : 整数型の範囲を超えました。\n";
static const char　NON_NUMBER_MSG[]　　= "error : 整数を入力して下さい。\n";
static const char　BAD_CHAR_MSG[]　　　= "error : 演算子を正しく入力して下さい。\n";
static const char　UNKNOWN_OP_MSG[]　　= "error : '+' '-' '*' '/'のいずれかの演算子を入力して下さい。\n";
static const char　USAGE[]　　　　　　 = "usage : %s lop op rop\n";

/** 四則演算を行う */
int
main(int argc, char *argv[])
{
　int 　　lop = 0, rop = 0, result = 0, mod = 0;
　char　op　= 0;

　check_ok_or_exit(argc != 4, 0, USAGE, argv[0]);

　lop　　= get_number_from(argv[1]);
　rop　　= get_number_from(argv[3]);
　op 　　= get_char_from(argv[2]);

　result　= calc(lop, op, rop, &mod);
　print_result(lop, op, rop, result, mod);
　return 0;
}

/** 四則演算 */
static int
calc(int lop, char op, int rop, int* mod)
{
　int　result　=0;
　assert(mod != NULL);
　*mod　= 0;　/* 除算以外は0を保証(print_resultで"余り"をはじく) */

　switch(op)
　{
　case '+':　result　= add(lop, rop);　　break;
　case '-':　result　= sub(lop, rop);　　break;
　case '*':　result　= mul(lop, rop);　　break;
　case '/':　result　= div(lop, rop, mod);　break;
　default :　check_ok_or_exit(ALWAYS_ERROR, -1, UNKNOWN_OP_MSG);　return -1;　/* no return */
　}
}

/** 数式表示 */
static void
print_result(int lop, char op, int rop, int result, int mod)
{
　printf("%d %c %d = %d", lop, op, rop, result);
　if(mod)　printf("余り%d\n", mod);
}

/** 十進表示数字(文字列)⇒int */
static int
get_number_from(const char* const msg)
{
　char*　check　= NULL;
　int　　value　= strtol(msg, &check, 10);
　check_ok_or_exit(errno == ERANGE, -1, OUT_OF_RANGE_MSG);
　check_ok_or_exit(*check != '\0', -1, NON_NUMBER_MSG);
　return value;
}

/** 文字列⇒単一文字 */
static char
get_char_from(const char* const msg)
{
　assert(msg != NULL);
　check_ok_or_exit(strlen(msg) >= 2, -1, BAD_CHAR_MSG);
　return msg[0];
}

/** 加算 */
static void
add(calc(int lop, int rop)
{
　check_ok_or_exit((lop > 0 && rop > 0) && (lop > INT_MAX - rop), 0, OVERFLOW_MSG);
　check_ok_or_exit((lop < 0 && rop < 0) && (lop < INT_MIN - rop), 0, OVERFLOW_MSG);

　return lop + rop;
}

/** 減算 */
static void
sub(int lop, int rop)
{
　check_ok_or_exit((lop > 0 && rop < 0) && (lop > INT_MAX + rop), 0, OVERFLOW_MSG);
　check_ok_or_exit((lop < 0 && rop > 0) && (lop < INT_MIN + rop), 0, OVERFLOW_MSG);

　return lop - rop;
}

/** 乗算 */
static void
mul(int lop, int rop)
{
　// "0の掛け算"をすると以下のゼロ除算で落ちるとかね
　check_ok_or_exit(lop > INT_MAX / rop, -1, OVERFLOW_MSG);
　check_ok_or_exit(rop < INT_MAX / lop, 0, OVERFLOW_MSG);

　return lop * rop;
}

/** 除算 */
static void
div(int lop, int rop, int* mod)
{
　assert(mod != NULL);
　check_ok_or_exit(rop == 0, -1, ZERO_DIV_MSG);
　check_ok_or_exit((lop == INT_MIN) && (rop == -1), -1, OVERFLOW_MSG);

　*mod = lop % rop;
　return lop / rop;
}

/**
* エラーチェック
* <p>
*　　条件に合致する場合、エラーとしてプログラムを終了する<br>
* 　　合致しない場合、何もしない
* </p>
* @param　expr　条件式
* @arg　　0(偽)　　何もしない
* @arg　　!0(真)　　プログラム終了(終了コードはerr)
* @param　err　q　エラー終了時の終了コード
* @param　msg　　エラー終了時の標準出力メッセージ(可変個引き数文字列)
*/
static void
check_ok_or_exit(int expr, int err, const char* const msg, ...)
{　
　va_list　va;
　va_start(va, msg);
　if(expr)
　{
　　vprintf(msg, va);
　　va_end(va);
　　exit(err);
　　return;
　}
　va_end(va);
}

回答No.4

MrBan

失礼。
> check_ok_or_exit(strlen(msg) > 2, -1, BAD_CHAR_MSG);
check_ok_or_exit(strlen(msg) >)= 2, -1, BAD_CHAR_MSG);ですね。

回答No.3

MrBan

一番単純な関数化の方針は「重複部分を削っていく(同じことは二度書かない)」
後は、「冗長なだけで意味のないコメントは書かない」
例：argv[2]に入力された演算子が'-'の場合
「意味のある名をつける」

で、ちょっとだけ弄ってみた一例
(一部のバグも直ってるし元コードとは仕様も多少違うけど…)
(これを推奨とは言わない…ゼロから書くならこうは書かないし…)

/* エラーが起きるとexitで抜けてしまう設計 */
#include <stdarg.h>
#include <string.h>
#include <stdlib.h>
#include <stdio.h>
#include <assert.h>
#include <limits.h>

/* $$$ プロトタイプ宣言省略 $$$ */

#define ALWAYS_ERROR　1　/* 常に失敗 */

/* エラーメッセージ */
static const char　OVERFLOW_MSG[]　　　= "error : オーバーフローが起こります。\n";
static const char　ZERO_DIV_MSG[]　　　= "error : 0による除算は禁止です。\n";
static const char　OUT_OF_RANGE_MSG[]　= "error : 整数型の範囲を超えました。\n";
static const char　NON_NUMBER_MSG[]　　= "error : 整数を入力して下さい。\n";
static const char　BAD_CHAR_MSG[]　　　= "error : 演算子を正しく入力して下さい。\n";
static const char　UNKNOWN_OP_MSG[]　　= "error : '+' '-' '*' '/'のいずれかの演算子を入力して下さい。\n";
static const char　USAGE[]　　　　　　 = "usage : %s lop op rop\n";

int
main(int argc, char *argv[])
{
　int 　　lop = 0, rop = 0, result = 0, mod = 0;
　char　op　= 0;

　check_ok_or_exit(argc != 4, 0, USAGE, argv[0]);

　lop　　= get_number_from(argv[1]);
　rop　　= get_number_from(argv[3]);
　op 　　= get_char_from(argv[2]);

　result　= calc(lop, op, rop, &mod);
　print_result(lop, op, rop, result, mod);
　return 0;
}

static int
calc(int lop, char op, int rop, int* mod)
{
　assert(mod != NULL);

　*mod　= 0;　/* 除算以外は0を保証(print_resultで"余り"をはじく) */
　switch( op )
　{
　case '+':　overflow_add(lop, rop);　return lop + rop;
　case '-':　overflow_sub(lop, rop);　return lop - rop;
　case '*':　overflow_mul(lop, rop);　return lop * rop;
　case '/':　overflow_div(lop, rop);　*mod = lop % rop;　return lop / rop;
　default :　check_ok_or_exit(ALWAYS_ERROR, -1, UNKNOWN_OP_MSG);　return -1;　/* no return */
　}
}
static void
print_result(int lop, char op, int rop, int result, int mod)
{
　printf("%d %c %d = %d", lop, op, rop, result);
　if(mod)　printf("余り%d\n", mod);
}

static int
get_number_from(const char* const msg)
{
　char*　check　= NULL;
　int　　value　= strtol(msg, &check, 10);
　check_ok_or_exit(errno == ERANGE, -1, OUT_OF_RANGE_MSG);
　check_ok_or_exit(*check != '\0', -1, NON_NUMBER_MSG);
　return value;
}
static char
get_char_from(const char* const msg)
{
　assert(msg != NULL);
　check_ok_or_exit(strlen(msg) > 2, -1, BAD_CHAR_MSG);
　return msg[0];
}
static void
overflow_add(int lop, int rop)
{
　check_ok_or_exit((lop > 0 && rop > 0) && (lop > INT_MAX - rop), 0, OVERFLOW_MSG);
　check_ok_or_exit((lop < 0 && rop < 0) && (lop < INT_MIN - rop), 0, OVERFLOW_MSG);
}
static void
overflow_sub(int lop, int rop)
{
　check_ok_or_exit((lop > 0 && rop < 0) && (lop > INT_MAX + rop), 0, OVERFLOW_MSG);
　check_ok_or_exit((lop < 0 && rop > 0) && (lop < INT_MIN + rop), 0, OVERFLOW_MSG);
}
static void
overflow_mul(int lop, int rop)
{
　check_ok_or_exit(lop > INT_MAX / rop, -1, OVERFLOW_MSG);
　check_ok_or_exit(rop < INT_MAX / lop, 0, OVERFLOW_MSG);
}
static void
overflow_div(int lop, int rop)
{
　check_ok_or_exit(rop == 0, -1, ZERO_DIV_MSG);
　check_ok_or_exit((lop == INT_MIN) && (rop == -1), -1, OVERFLOW_MSG);
}
static void
check_ok_or_exit(int expr, int err, const char* const msg, ...)
{　
　va_list　va;
　va_start(va, msg);
　if(expr)
　{
　　vprintf(msg, va);
　　va_end(va);
　　exit(err);
　　return;
　}
　va_end(va);
}

お礼

<<「冗長なだけで意味のないコメントは書かない」
そうですね。関数以前に無駄なものは省かないと駄目ですね。

main関数はスッキリしますが、やはり理解するのが難しいですね。
なぜかまた掛け算が上手くいかなくなってしまいました（泣）

回答No.1

ibe_tattsu

前回の質問も読ませていただいておりました。
大変苦労なさっているようですね。

今回、関数化されたいようですが、何をお困りでしょうか？
個人によって好みや考えが違い、関数や記述の仕方が変わってきます。
現段階の時点で、関数を使われているので書き方が分からないということではないでしょうが。

どう関数化すればプログラムがすっきりするかということですかね？
長くなったプログラムを関数やモジュールとして分割したいのであれば、基本的に各機能や処理ごと。
一旦、処理の流れをフローチャートなどにして全体を見てみると、
全体の処理を把握することができ、どのように分割したらきれいになるかがわかるかもしれません。

色々大変でしょうが、がんばってくださいね。

お礼

<<どう関数化すればプログラムがすっきりするかということですかね？
はい。その通りです。とにかくmain関数をスッキリさせたいです。特に
エラーチェックが多いので、まとめられたらと思います。

ちなみに少しだけ関数化させてみたのですが、あまり意味のない関数化
だと思い、全く自信がありません。おかしい部分を指適して下さると
ありがたいです。あと、こうした方がいいというアドバイスがあれば
よろしくお願いします。