低次元なコーディング方法について

質問

私は一通り書籍でC言語の基礎を勉強しただけの初心者プログラマです。
もっと実力を付けるためにアルゴリズム関連の書籍をいくつか読み始めました。
これならソースを読む力も一緒に鍛えられると思ったからです。
その中で素数を表示するプログラムに興味を持ち自力で一から組んでみました。
(途中でどうしても分からず参考にする部分もあったのですが…)

私のコーディング手順は
1.UMLのアクティビティ図で大まかな処理を確認する。
2.if文だけで一通りの処理を記述。
3.while文を使って繰り返しの処理を追加する。
4.何度もうまくいかずにそのたびに修正していく。
というものです。

1.の部分は大量に書籍が出ているので勉強できるのですがUMLの性質上
それをコーディングしていく解説はどうしてもCのソースを書くこととは次元が違うようです。
かなり低次元なことだからでしょうか、解説している本にも出会えません。

前置きが長くなりました。
今回教えていただきたいのは
・アクティビティ図などで記述したものをソースコードに落とし込む
　というかなり低レベルな部分での方法です。
私はオブジェクト指向はまだ勉強していないので
Cでその過程などを書いていただけると幸いです。
どうやったらgoto文やフラグを使わずにソースを書けるか、
コツなどあったらぜひ教えてください。

アドバイスの参考になればよいと思い、私が書いた素数表示プログラムの一部を紹介させていただきます。

/**********************************************************/
/* 素数を格納する配列を受け取り最後に-1を格納する関数　　*/
/*　効率的な演算のために乗除演算回数を表示もおこなう　　　*/
/**********************************************************/

#define　　END (-1)
#define MAX 1000

void prime_assess(int a[])
{
　　int j=0;　　//ループ用のカウンタ
　　int x=3;　　//検証値
　　int element_no=1;//配列の中の素数の個数
　　int arithmetic_no=0;//乗除演算回数

　　while(x<MAX){
　　　　j=0;
　　　　arithmetic_no++;
　　　　while(x % a[j] !=0 && a[element_no-1] != x){
　　　　　　arithmetic_no++;
　　　　　　if(a[j]-a[j] >= x){
　　　　　　　　a[element_no++] = x;
　　　　　　}else{
　　　　　　　　j++;
　　　　　　}
　　　　}
　　　　x += 2;
　　}
　　a[element_no] = END;
　　printf("乗除演算回数は%dでした。\n",arithmetic_no);
}

先ほどの1～4のプロセスで作りました。
できるだけ無駄な演算を避けてみたつもりですがいかかでしょうか。
フラグなどを避けるよう、かなり時間をかけて書いたプログラムです。
乗除演算回数はおそらく2636回になっています。
自分以外の方にソースを見ていただける機会がないものですから、
これはおかしいぞというところも書いていただけたらうれしいです。

皆様からのアドバイスをお待ちしております。

回答No.7

aid-u · ベストアンサー

> ・アクティビティ図などで記述したものをソースコードに落とし込む
> 　というかなり低レベルな部分での方法です。
アクティビティ図の使用は、しばらく止めてみてはいかがでしょうか。

処理の流れを書いたり、考え方を整理するのに使われるものとしてフローチャートがあります。
フローチャートはご存知だと思います。
フローチャートに対する批判として、記述の自由度が高い(矢印をどこへでもつなぐことができる)ため
スパゲッティコードになってしまうというものがあります。

アクティビティ図で処理の流れを記述するのも同様の問題があるのだと思います。
stafieさんが悩んでいるのも、この点ではないでしょうか。

アクティビティ図の代わりとしては、疑似コードを使用してみるのが良いと思います。

参考URLは疑似コードについて説明したものです。
Java言語について書かれたものですが、疑似コードの理解には問題無いと思います。

参考URL：

http://java2005.cis.k.hosei.ac.jp/materials/lecture10/method4.html

お礼

aid-uさん、ありがとうございます。
HPを見ながら擬似コードの使い方が分かってきました。
サンプルがJavaでテーマも再帰を使っていたので
後半は苦労しましたが。^_^}
私自身がフローチャートやUMLばかりにこだわりすぎていたのが
よくなかったのですね。

もしC言語を主に使って擬似コードを書いたり、そこからコーディングしていくような内容の書籍　(私が調べたら「珠玉のプログラミング」が一番その要望に沿っているようでした)　が合ったらぜひ教えてください。

回答No.8

aid-u

ANo.7です。

疑似コードに関する書籍ですが、何をお勧めして良いかわかりません。

私が疑似コードに関して最初に目にした本は「ソフトウェア作法」だったと思いますが、これは今となっては内容が古いと思います。

「珠玉のプログラミング」は、評判の良い書籍なので見てみると良いと思います。
ただし、疑似コードに関する記述量は期待されているほどでは無いかもしれません。

アルゴリズム関係の本でも疑似コードを使用してアルゴリズムを説明しているものがあるようです。
このような書籍を調べてみるのも良いと思います。

お礼

aid-uさん　ありがとうございました。
自分でも調べてみます。:-)

併せて擬似コードについて記述していただいたaid-uさんに良解答を付けさせていただきます。
ありがとうございました。:-)

回答No.6

plh

「実力を付ける」ということが目的のようですので、ANo.4 の補足のコードに敢えて意地悪っぽく意見を述べるのなら、次のようになります。
・prime_assess() に引数 int a[] を渡しているが、これだけでは配列のサイズが不明なので、サイズを同時に渡すべきである。
　　　　例えば void prime_assess(int aiPrimeNumber[], int iArraySize); のように。
　MAX_PRIME_NUMBER が #define されているからいいじゃないかという考え方もありますが、これでは prime_assess() が一人歩きできません。
・また、変数名 a はこれだけでは意味不明なので、素数の array of integer の意味で上記では aiPrimeNumber としました。このように変数の命名規則を自分なりに決めておくととても見通しが良くなります。
・そういう意味では、j_loop_counter の j は意味不明なので、iLoopCounter の方が良いでしょう。
　更に言えば、これは意味としてはループカウンタではなく、素数を入れる配列のインデクスなので、iPrimeNumberIndex とでもする方が良いでしょう。
　_ を使うのが好きなら、i_prime_number_index でもよいですが、これは趣味の問題です。
　同様に x はこれは素数の候補ですから iPrimeNumberCandidate でしょう。最初の i は int の意味です。
　同様に element_no は素数の個数なので、iPrimeNumberCount とか。
　同様に arithmetic_no は演算回数なので、iArithmeticCount とか。
　要するにコメントをつけるまでも無く変数名を見ればその変数の型と意味が自明になるように努力するということです。
　C や C++ は型に厳格な言語ですので、変数型が一目で分かることにはプログラミング上の明らかな利点になります。
・j_loop_counter は、while の中でしか使っていないようなので、while {} の中に次のように局所化しましょう。
　　while (...) {
　　　　int iLoopCounter = 0;
　　　　:
　　}
　変数はできる限り局所で使用し、使用後は直ちに自動消滅させるというのが基本です。
・デバグ目的ならかまいませんが printf を prime_assess() の中で使うと prime_assess() が一人歩きできませんので、毎回これらの数値を得る必要があるなら、prime_assess() から返して main() 側で表示するようにしましょう。
　次のような感じです。
　　void prime_assess(int aiPrimeNumber[], int iArraySize, int* piPrimeNumberCount, int* piArithmeticCount) {
　　　　:
　　　　if (piPrimeNumberCount) { *piPrimeNumberCount = iPrimeNumberCount; }
　　　　if (piArithmeticCount ) { *piArithmeticCount　= iArithmeticCount ; }
　　}
　ちなみに pi は pointer to integer の意味で使っています。
　返す値が一つだけなら int prime_assess(); として、値を返すのが良いでしょう。
・main() の中で int prime[MAX_PRIME_NUMBER/2+1] ={2}; はいただけません。「素数を求める」という機能が main() と prime_assess() の両方に部分的に実装されていることになってしまいます。
　つまり、素数 2 を配列の最初に入れているこの main() でなければ prime_assess() はまともに動かないという制約を作っていしまっているということで、prime_assess() が一人歩きできません。

回答No.5

Interest

プログラムを書く力を養うために、
１．良いソースコードを読むこと
２. 自分で書いてみること
３．他人に見てもらうこと
は非常に重要なことだと思います。これって、プログラムだけでなく、文章を書く技術でも同じことですよね。

コーディング手順について書いてらっしゃいますが、コーディング（実装）する前に「仕様決め」と「設計」という工程が重要です。（そして実装後には「テスト」という工程もありますが、まだ触れなくていいかも）

まずは「仕様決め」を今回の素数を求めるプログラムで考えてみます。簡単な例を挙げます。
* 入力は何？　値の範囲は？
* 出力は何？　入力に対してどのような出力が出ればよい？
* 不正な入力をした場合はどうする？

これに照らすと、stafieさんの素数生成関数prime_assessの仕様はどうなっているでしょうか。
(疑問 1) 配列 a は入力？　それとも出力？
(疑問 2) 関数の呼び出し元で配列 a のサイズが int a[3] 程度しか確保されていなかったら？

続いて設計工程では、仕様を実現するための仕組みを考えます。その考えるための道具が、フローチャートだったり、アクティビティ図だったりします。この工程では使用するデータ構造やアルゴリズムの概念を図示できれば十分で、必ずしもソースコードの各行と一致する必要はありません。紙と鉛筆で処理の流れを追って入力に対する出力が先に決めた仕様通りに出てくることを確認できれば、それで必要十分な設計だと思います。

UMLは主に仕様決めから設計で使用する道具ですが、今の段階ではUMLにあまりこだわらない方が良いでしょう。UMLはオブジェクト指向でソフトウェア開発を行うことを前提にしているため、アルゴリズムをC言語のソース付きで解説しているモノには馴染みません。

> アクティビティ図などで記述したものをソースコードに落とし込む
> Cでその過程などを書いていただけると幸いです。

実装工程で「設計をコードに落とし込む」わけですが、言われてみると私も何か方法論があるわけでもなく、なんとなく書いていることに気付きました。強いて挙げるなら、こんな感じでしょうか。
１．エラー処理を先に書く。
２．できるだけ素直に書く。（変なテクニックを持ち込むと、あとで読みづらくなるし、バグの温床になる。）
３．「慣用表現」や「定石」はそのまま使う。（良いソースコードを読んでいるうちに、お決まりのパターンがわかってくる。）

> どうやったらgoto文やフラグを使わずにソースを書けるか、

必ずしも gotoやフラグが悪というわけではないので、使うべき場所さえ間違えなければ使っていいと思いますよ。gotoをよく見かける場所は、エラー処理等で関数を抜ける前に常に実行すべき処理がある場合などです。フラグはプロが仕事で書くプログラムであっても無くならない（笑）

回答No.4

asuncion

> 素数を格納する配列

これはあれか…。
素数を格納する「ための」配列、と読み解かねばならなかったですね。
で、他のかたの回答にもありますとおり、

> if(a[j]-a[j] >= x){

ここは写し間違いか何かでありましょう。
最新版のソースコード全体（一部だけではなくて）をコピー＆ペーストして見せてくだされば、
どこをどうすればよいかがわかるかもしれません。

補足

ご指摘をいただいた部分を改良した最新版ソース全体です。
よろしくお願いします。

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

#define MAX_PRIME_NUMBER 1000
#define END　　(-1)

/**********************************************************/
/*　素数を格納する配列を受け取り最後に-1を格納する関数　　*/
/*　効率的な演算のために乗除演算回数を表示もおこなう　　　*/
/**********************************************************/
void prime_assess(int a[])
{
　　int j_loop_counter=0;　　//ループ用のカウンタ
　　int x=3;　　//検証値
　　int element_no=1;//配列の中の素数の個数
　　int arithmetic_no=0;//乗除演算回数

　　while(x<MAX_PRIME_NUMBER){
　　　　j_loop_counter=0;
　　　　arithmetic_no++;
　　　　while(x % a[j_loop_counter] !=0 && a[element_no-1] != x){
　　　　　　arithmetic_no++;
　　　　　　if(a[j_loop_counter]*a[j_loop_counter] >= x){
　　　　　　　　a[element_no++] = x;
　　　　　　}else{
　　　　　　　　j_loop_counter++;
　　　　　　}
　　　　}
　　　　x += 2;
　　}
　　a[element_no] = END;

　　printf("1000以下の素数は%d個です。\n",element_no);
　　printf("乗除演算回数は%dでした。\n",arithmetic_no);

}

/**********************************************************/
/* main プログラム */
/**********************************************************/

int main(void)
{
　　int i_loop_counter;
　　int prime[MAX_PRIME_NUMBER/2+1] ={2};

　　prime_assess(prime);

　　i_loop_counter=0;
　　while(prime[i_loop_counter] != END){
　　　　printf("%4d = %3d\n",i_loop_counter+1, prime[i_loop_counter]);
　　　　i_loop_counter++;
　　}

　　return (EXIT_SUCCESS);
}

回答No.3

plh

まず、if(a[j]-a[j] >= x){ ってのは、冗談ですよね。本当に素数が求まっていますか?
「もっと実力を付けるために」であれば、UML というよりは、まず、変数名をもうすこしわかりやすくするなどを工夫すべきだと思います。
例えば
int j=0 // ループカウンタ
ではなく、
int iLoopCounter = 0;　　// コメントは不要
とするとかですね。
MAX もそれだけを見て何の最大値か分かるように MAX_PRIME_COUNT とするとかなどです。
本当に本気で実力を付けたいのであれば、C++ を学んで、結果を vector<int> で返すなどして、危険なポインタ(int a[])を使わないですむようにするとか、やるべきことはたくさんあります。
また、goto 文はともかく、フラグを使ってはいけない理由はあまり無いと思います。
参考までに C++ ではほぼ次のようになります。VC++6 でコンパイル可能です。

#include <cstdio>
#include <vector>
using namespace std;

vector<int> PrimeNumbers(int iMaxNumber) {
　　vector<int> veciPrimeNumber;　　// 素数を入れる配列
　　{for (int iPrimeCandidate = 2; iPrimeCandidate < iMaxNumber; iPrimeCandidate++) {
　　　　bool bIsPrime = true;
　　　　{for (int iIndex = 0; iIndex < veciPrimeNumber.size(); iIndex++) {
　　　　　　if (iPrimeCandidate % veciPrimeNumber[iIndex] == 0) {
　　　　　　　　bIsPrime = false;
　　　　　　　　break;
　　　　　　}
　　　　}}
　　　　if (bIsPrime) {
　　　　　　veciPrimeNumber.push_back(iPrimeCandidate);
　　　　}
　　}}
　　return veciPrimeNumber;
}

int main() {
　　const int iMaxNumber = 20;　// 素数を探す整数の最大値
　　vector<int>& rveciPrimeNumber = PrimeNumbers(iMaxNumber);
　　{for (int iIndex = 0; iIndex < rveciPrimeNumber.size(); iIndex++) {
　　　　printf("%d\n", rveciPrimeNumber[iIndex]);　 // 素数をプリントする。
　　}}
　　return 0;
}
健闘を祈ります。

回答No.2

noname#246547

＞2.if文だけで一通りの処理を記述。
＞3.while文を使って繰り返しの処理を追加する。
オブジェクト指向は勉強されていないとのことなので、構造化プログラミングの考え方からすると、
先にif文書いて次にwhile文を追加で書くやり方は賛成できません。
今回のように単純な仕様なら良いですが、複雑な仕様の場合たぶんコードが作れないか、バグだらけです。たとえばwhile内が1つの意味のある処理の塊であるならば関数化して、これを呼び出すようにすれば、while内は簡略化できるでしょう。

＞どうやったらgoto文やフラグを使わずにソースを書けるか
そもそもアクティビティ図が完成しているなら、機械的にコード化出来るはずです。
図内の手続きは四則演算等に置き換わるはずだし、分岐判断はif文（または、switch,goto等)に変わるはずだし、ループになっているところはwhile（まはたfor等）文に単純に置き換えていくだけのはず。

フラグを使用するかgotoを使用するかどんな変数をしようするかはアクティビティ図内で決定されているはずですよ。
どうしてもgotoを使用してしまうのであれば、機能ごとに関数化して呼び出すようにしてコードをすっきりさせるといいかと。
フラグを少なくとありますが、フラグは無理に減らさなくてもいいと思います。減らしたいのであれば、まずフラグは減らすことを考えずにコーディングし、次にコードを見直して無駄なフラグを減らせばよいでしょう。

次にコードですけど、
コードななめ読みで申し訳ないですが、これって正しく素数の一覧が出ました？
x % a[j] !=0 とか　a[j]-a[j]　っておかしくない？
前者は初めて実行されたときにa[j]の中身って不定では？
かりに、呼び元の関数で初期化していたとするとzero divideでは？
後者は演算結果が必ず０では？０ならば演算する意味なし。

補足

すみません。コードを乗せるときに見やすくなるように置換したときにミスをしてしまったようです。

以下がソースの全てです。

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

#define MAX 1000
#define END (-1)

void prime_as(int a[])
{
　　int j=0;　　//ループ用のカウンタ
　　int x=3;　　//検証値
　　int element_no=1;//配列の中の素数の個数
　　int arithmetic_no=0;//乗除演算回数

　　while(x<MAX){
　　　　j=0;
　　　　arithmetic_no++;
　　　　while(x % a[j] !=0 && a[element_no-1] != x){
　　　　　　arithmetic_no++;
　　　　　　if(a[j]*a[j] >= x){
　　　　　　　　a[element_no++] = x;
　　　　　　}else{
　　　　　　　　j++;
　　　　　　}
　　　　}
　　　　x += 2;
　　}
　　a[element_no] = END;

　　printf("1000以下の素数は%d個です。\n",element_no);
　　printf("乗除演算回数は%dでした。\n",arithmetic_no);

}

int main(void)
{
//　　int i;
　　int prime[MAX/2+1] ={2};

　　prime_as(prime);

/*
　　i=0;
　　while(prime[i] != END){
　　　　printf("%4d = %3d\n",i+1, prime[i]);
　　　　i++;
　　}
*/　　
　　return (EXIT_SUCCESS);
}

回答No.1

asuncion

> /* 素数を格納する配列を受け取り最後に-1を格納する関数　　*/

このコメントを素直に読むと、prime_assess関数の引数aには
あらかじめ素数群が入っているように思えます。
そうなのですか？

それはさておき、今回のプログラムにUMLとかアクティビティとか
必要なのでしょうか…。
私には必要性が理解できません。

補足

素数を格納"するため"の配列が適切でした。すみません。

aは先頭に2が入っていてそれ以外は0です。
No.4の方の補足にソース全てを掲示させていただきました。
そちらをご覧ください。

私はループの処理が重なってくるとすぐにこんがらがってしまうので
アクティビティ図を描きました。

asuncionさんはそういった場面でもどんどん組めるようですが
どうしたら私もそうなることができますか？
やっぱり経験とか才能なのでしょうか。