質問: strlen(s); for(i=0;i=prior(s[i]) && is_empty(head)==FAILURE){ //新たに読み取った文字の方が、スタック最上部の文字より優先度が低ければ、スタックをポップしてそれを読み取る。 p[j]=last_node(&head); pop(&head); j++; } //スタックの最上部の文字より新たに読み取った文字の優先順位が高ければ、スタックに積む if(prior(last_node(&head))
回答: こんにちは。 解析に軽くトライしてみましたが、う~ん、複雑すぎてギブアップです。 push()から呼び出したnew_node()でpush処理を実行されているようですが、push(a)->push(b)->pop()->pop()とした時、b, aの順序でちゃんとpop()されるでしょうか? pop()はリストを追跡してないですがnew_node()はリストを追跡しているようなのでスタックではなくキュー(待ち行列)になっているような印象を受けます。コメントも「ノードを後ろに付け加える」となってますし。push()のコメント「スタックの先頭へのノードの挿入」と矛盾している印象です。 ですので、どうもpush()/pop()が結構大きくバグっているような気がします。 逆ポーランドの前に、まずpush(), pop()をきちんと動かすことをお勧めします。 例えば、push(a), push(b), pop(), pop()してちゃんとb, aの順序で読み出せることのテストです。 その時、is_empty()もテストしましょう。 【余談ですが】 すいません、厳しいことを言います。ソース、なかなか読みにくいです。 回答がほとんどつかないのはそれが原因と思います。読みにくいプログラムを読むのは本当にたいへんなのです。 さて、インデントが1文字や2文字と言うのは初めてみました。インデントの数を場所によって変えるのはよろしくないです。また、1文字はいくらなんでも少なすぎます。最低2文字以上つけましょう。私が一番良くみるインデント量は4文字です。(2文字のソース、8文字のソースも時々見ます。) 変数名は意味のある名前をつけましょう。nとかpとかppとかではawellbottomさんの意図を読み取れません。 push/popの呼び出し回数を同じになるようにしましょう。回数がアンバランスな時、そこにバグが潜む可能性が高いです。 push()から呼び出されているnew_node()でheadを直接修正していますので、このプログラムで取り扱えるスタックはhead1つだけですね。であれば、ppパラメータはやめにして全てheadをアクセスすれば可読性が大きく改善しますよ。 headをパラメータとすることで複数のスタックを使えるようなプログラムを目指すことはたいへん好ましいことです。ですが、そのようなプログラムはデバッグの難易度も高くなりますし、可読性が落ちやすいので変数名の付け方等、可読性への配慮をより多くしないといけません。 自力でのデバッグを諦めるような時は、諦める前に抽象化を断念してデバッグすることをお勧めします。 それでもダメな時に初めて他の人に聞くと、抽象化されていない分把握しやすいプログラムになっていますので、他の人も回答しやすくなります。 最後に、initialize()のような1行関数は、他の人にデバッグを依頼する場合は、障害になります。 `head=NULL;`と書いてあればその場で把握できるのに、`initialize(&head);`と書かれているとinitialize()関数が何か悪さしてないか確認するために、ソースの上の方を見に行かないといけません。しかも、バグりやすい2重ポインタです。本当に間違ってないのか確認するために、デバッグ文を仕込む必要もあるかも知れません。なかなか痛いです。 【オブジェクト指向のススメ】 C言語で、複数のスタックを同時に使えるようにするためには、headをパラメータとして一々渡す必要があり、しかも、関数側でポインタの変更が必要なため、2重ポインタを使う必要が有ります。どうしても可読性/メンテナンス性が低下しやすいです。 しかし、C++言語のオブジェクト指向を使えば、C言語の性能をほとんど落とすことなく、必要な変数群とそれを操作する関数群をスタック・クラスとしてひとまとめにできます。一々headをパラメータとして渡すことなくスマート(2重ポインタなど使わず)にアクセスでき、しかも、複数のスタック実体を作れます。本当に便利ですよ。 汎用性のあるプログラムを開発するよう努力されているようですので、ある程度C言語を使いこなせるようになったら、C++言語も検討されることをお勧めします。