スタックのプログラムを作成しているのですが、うまく出来ません。

質問

プログラムの内容を簡単に言うと、配列s[100]と変数topを用いて、ファイルdata.datからgetc()を用いて文字を１文字ずつ読み込み、スタック（s[100]）にpush-downするプログラムです。
細かく言うと、作成したプログラムによりファイルからキー（文字）を読み込みスタックにpush-downし、スタックの内容を表示した後に、キーボードからキー（文字）を１文字ずつ入力して、スタックを操作する。
・スタック操作の仕様はキーボードからキー（文字）を１文字ずつ入力する際に、0を入力した場合、プログラム終了。１を入力した場合、１文字pop-upした後、pop-upした文字とスタックの内容を表示。その他の文字を入力した場合、その文字をpush-downした後、スタックの内容を表示。(スタックの内容の表示はprint_stack_mtrx(s,top)を使用する。)
・push-downとpop-upはそれぞれ１つの関数で定義する。
といった感じのプログラムを作成しているのですが、関数push,popの部分をどう書いたら良いのか良く分かりません。一応自分で書いてみたのですが、うまくいきませんでした。どなたか教えていただけないでしょうか？
*ファイルdata.datからはリダイレクションを用いて読み込む。
<作成途中のソースプログラム>
#include<stdio.h>
#include<stdlib.h>
#define MAX 100

char s[MAX];
int top;

void init_stack(){
______top = 0;
_______return;
}
void print_stack_mtrx(char* s, int top){
__int i;
____if(top == 0){
______printf("Stack is empty.\n");
____}
____else{
______printf("--- Contents of Stack ---\n");
______for(i = 0; i < top; i++){
________if(!i){
__________printf("%2c < -- Top (%2d)\n", s[top - i - 1], top);
________}
________else{
__________printf("%2c \n", s[top - i - 1]);
________}
______}
______printf("-------------------------\n");
____}
}
char push(char* s, int top, char j){
___s[top]=j;
___top++;
}
char pop(char* s, int top){
_char c;
___c = s[top];
___top--;
___return c;
}

int main(void)
{
_int c;
_char j;
_char i;

init_stack();
_______while(((c=getc(stdin))!=EOF) && top<MAX){
_______/* ファイルdata.dat からgetc()を用いて1文字ずつ読み込みスタックsに格納．ただしスタックの出入り口を示す top の値も監視すること*/
_________s[top] = c;
_________top++;
_______}

_______print_stack_mtrx( s, top );/* スタック（配列）の内容を表示する関数*/
_______while(1){
_________scanf("%c\n", &j);
_________if(j=='0'){
___________break;
_________}
_________else if(j=='1'){
___/* 1文字pop-upした後, pop-upした文字とスタックの内容を表示 */
___________i = pop(s, top);
___________printf("pop-upした文字: %c\n", i);
___________print_stack_mtrx( s, top );
_________}
_________else{
___/* その他の文字を入力した場合, その文字をpush-downした後,スタックの内容を表示 */
___________push(s, top, j);
___________print_stack_mtrx( s, top );
_________}
_______}
_______return 0;
}

回答No.6

kameDK

いくつか問題点を・・・

まずpopですが、popする関数ではなくpopできるか確認するだけのプログラムになってしまっています。折角jに取り出した文字を格納しているので、popした文字を表示する場合はそのjを用いた方がいいと思います。

これはただの表示の問題です、スタックの中身を表示している部分で、topが指している位置が違います。スタックのイメージを確認してみるといいと思います。

それから、補足で実行結果がありますがよく見てみると1回の入力に対してスタックの内容が2回表示されていませんか？コードを見る限りそのように組んでいるようには見えません。プログラムが思ったとおりに動かないのはこのせいですね。scanfについて調べてみると解決すると思います。

最後に・・・このプログラムでは０と１だけスタックに積むことができませんがそれはかまわないのでしょうか？

頑張ってください。

補足

＞popした文字を表示する場合はそのjを用いた方がいいと思います。
変更しました。
＞topが指している位置が違います。
具体的にはどうすれば良いでしょうか？
scanfはあまり使わないほうがよいと調べてたら書いてあったので、scanf("%c\n", &j);→j=getchar();に変更しました。
＞このプログラムでは０と１だけスタックに積むことができませんがそれはかまわないのでしょうか？
かまわないです。

while(1)中のif文で、そこのelseの中のelseのprint_stack_mtrx( s, top );を消してコンパイルして実行したら0と1が入力された場合はちゃんと出力したので、この結果から、elseの中のelseのprint_stack_mtrx( s, top );がなぜか実行されていることがわかりました。
なぜ、if文の条件を無視してprint_stack_mtrx( s, top );が実行されているのでしょうか？

回答No.5

Lchan0211

参照渡し/値渡しの問題もありますが、それ以前に、
print_stack_mtrx(),push(),pop()の各関数引数で
sやtopを渡そうとしていること自体が誤っています。

s,topは、スタック内部で管理する変数です。
main関数で直接s,topを使用してはいけません。

入れた順と逆順にお金を取り出せる貯金箱(スタック)を
作ったはずが、入れる位置や取り出す位置や入っている量を
使う側から指定しなければならないのだったら、貯金箱(スタック)の
意味がありません。

回答No.4

kameDK

pushではchar型配列s、int型のtop, char型のj。
popではchar型配列s、int型のtopですよね。

確かにその通りです。

お聞きしたかったのは、それぞれの型ではなくデータがどのように受け渡され扱われるかということです。
私の日本語が下手でうまく伝えられず申し訳ないです。

int型やchar型のようなデータを引数とした場合、その値が引数として渡されます。
pop関数で見てみると・・・
char pop(char* s, int top)
この場合、引数としてcharのポインタとintの値を受け取ります。
そこでpopは新たにchar* sとint topという変数を宣言して受け取ったcharのポインタ、intの値を代入します。
つまりmainのtopとpopのtopは同じ値、同じ名前ですが違う変数です。
ですので、popの中でいくらtopの値を変更してもmainのtopの値は変更されません。
その結果いくらpopしてもmainの中のtopの値が変更されず同じ表示になってしまっています。

じゃあpopの中でmainのtopの値を変更するにはどうしたら？となると思います。
既に知っているかもしれませんが、ポインタについて勉強するといいかと思います。

きっとうまく説明できていないかと思います。
以下のページが図などを用いて上手く説明されていますので参考にしてみるといいかもしれません。
今回は参照渡しにすべきところを値渡しにしてしまっている為に思った動きをしてくれていないですね。
http://ufcpp.net/study/csharp/sp_ref.html#byval

補足

下記のようにソースを変更しました。
#include<stdio.h>
#include<stdlib.h>
#define MAX 100
char s[MAX];
int top;
void init_stack(){
top = 0;
return;
}
void print_stack_mtrx(char* s, int top)
{
int i;

if(top == 0){
printf("Stack is empty.\n");
}
else{
printf("--- Contents of Stack ---\n");

for(i = 0; i < top; i++){
if(!i){
printf("%2c < -- Top (%2d)\n", s[top - i - 1], top);
}
else{
printf("%2c \n", s[top - i - 1]);
}
}

printf("-------------------------\n");
}
}
char push(char j){
if(top >= MAX)
return (-1);
s[top] = j;
top++;
return (0);
}
char pop(char *j){
if(top <= 0) /* スタックは空 */
return (-1);
top--;
*j = s[top];
return (0);
}
int main(void)
{
int c;
char j;
FILE *fp;
if((fp=fopen("data.dat","r"))==NULL) {
printf("ファイルが見つかりません: data.dat\n");
exit(EXIT_FAILURE);
}

init_stack();
while(((c=getc(fp))!=EOF) && top<MAX){
/* /kyozai/mitsuda/prog2/data.dat からgetc()を用いて
1文字ずつ読み込みスタックsに格納．
ただしスタックの出入り口を示す top の値も監視すること */
s[top] = c;
top++;
}
print_stack_mtrx( s, top );
/* この関数は/kyozai/mitsuda/prog2/print_stack_mtrx.o
にあり，スタック（配列）の内容を表示する関数である．*/
while(1){
scanf("%c", &j);

if(j=='0'){
break;
}
else if(j=='1'){
/* 1文字pop-upした後, pop-upした文字とスタックの内容を表示 */
printf("pop-upした文字: %c\n", s[--top]);
if(pop(&j) == -1)
puts("ポップできません。");
else print_stack_mtrx( s, top );
}
else{
/* その他の文字を入力した場合, その文字をpush-downした後,
スタックの内容を表示 */
if(push(j) == -1)
puts("スタックへのプッシュに失敗しました。");
else print_stack_mtrx( s, top );
}
}
fclose(fp);
return 0;
}
コンパイルして実行すると、0を入力した場合は何事もないのですが、1を入力すると↓のようになってしまいます。
＜実行例＞
--- Contents of Stack ---
R < -- Top ( 8)
E
T
U
P
M
O
C
-------------------------
1
pop-upした文字: R
--- Contents of Stack ---
T < -- Top ( 6)
U
P
M
O
C
-------------------------
--- Contents of Stack ---

< -- Top ( 7)
T
U
P
M
O
C
-------------------------
(入力待ち)
というようになってしまいます。
その他の文字でコンパイルすると
--- Contents of Stack ---
R < -- Top ( 8)
E
T
U
P
M
O
C
-------------------------
a/* aを入力 */
--- Contents of Stack ---
a < -- Top ( 9)
R
E
T
U
P
M
O
C
-------------------------
--- Contents of Stack ---

< -- Top (10)
a
R
E
T
U
P
M
O
C
-------------------------
b/* bを入力 */
--- Contents of Stack ---
b < -- Top (11)

a
R
E
T
U
P
M
O
C
-------------------------
--- Contents of Stack ---

< -- Top (12)
b

a
R
E
T
U
P
M
O
C
-------------------------
(入力待ち)
というようになってしまいます。どうすれば改善できるか教えていただけないでしょうか？

回答No.3

kameDK

答えを書いてしまうとためにならないかと思いますのでヒントだけ・・

関数が呼び出され引数が渡されるとき、どんなデータが受け渡されているか分かりますか？
mainの中とpopの中でそれぞれtopのアドレスを表示してみてください。
違いが分かるかも？

補足

pushではchar型配列s、int型のtop, char型のj。
popではchar型配列s、int型のtopですよね。

回答No.2

Yanch

自作関数のアーキテクチャについて理解してないと推測します。
関数の動作について復習してみてはいかがでしょう。

参考になりそうなページ
http://www.geocities.jp/ky_webid/c/008.html

後は、トレースをがんばれば、問題個所は見えてくるでしょう。

回答No.1

kameDK

とりあえず・・・push,popについてですが

char push(char* s, int top, char j){
___s[top]=j;
___top++;
}
char pop(char* s, int top){
_char c;
___c = s[top];
___top--;
___return c;
}

↓

char pop(char* s, int top){
_char c;
___c = s[--top];
___return c;
}
pushのほうを見ると変数topは次にスタックを積むべき場所を保持していると思います。topの位置にはまだ実際にはデータが入ってないということになります。
しかしpopではそのまだ実際にデータが入っていない位置からデータを取り出そうとしています。popではプレデクリメントが必要になるかと思います。

お礼

回答ありがとうございます。
kameDKさんの言うとおりにpop関数を変えました。
無限ループの条件分岐の中の"1文字pop-upした後, pop-upした文字とスタックの内容を表示"で、スタックの内容の表示が
while(((c=getc(stdin))!=EOF) && top<MAX){
_________s[top] = c;
_________top++;
_______}
_______print_stack_mtrx( s, top )
でスタックの内容の表示したものになってしまいます。
それと "その他の文字を入力した場合, その文字をpush-downした後,スタックの内容を表示"の部分でも上記と同様になってしまいます。考えてはみたのですが、どう書いたらちゃんと出力するのかよくわかりません。よかったらおしえていただけないでしょうか？