weemiee
@weemiee (weemiee)

Are you sure you want to delete the question?

If your question is resolved, you may close it.

Leaving a resolved question undeleted may help others!

We hope you find it useful!

スタック・pushとpopをvoid型にしたい

Discussion

スタックについてです。ある問題を模範解答通りに解いた後で少し弄ってみて、そこで行き詰ってしまいました。元の問題では、push関数とpop関数をint型で定義していたので、今までによく見てきたvoid型で書き換えられないかと思い、ひとまず自力でプログラムを考えました。

#include <stdio.h>
#define N 10

typedef struct vehicle{
	char stach[N];
	int size;
}carEtc;

carEtc Stack;

void init(void){
	Stack.size = 0;
}

void push(Stack *stack){
	int input;
	if (stack->size >= N) {
		printf("failed\n");
	}
	stack->stach[stack->size] = input;
	stack->size++;
}

void pop(Stack *stack){
	int *output;
	if (stack->size <= 0) {
		printf("failed\n");
	}
	*output = stack->stach[stack->size - 1];
	stack->size--;
}

int main(void){
	char cartype1[] = "自動車";
	char cartype2[] = "原動機付自転車";
	char cartype3[] = "軽車両";
	char cartype4[] = "路面電車";
	push(&cartype1);
	push(&cartype2);
	push(&cartype3);
	push(&cartype4);
	pop(&cartype1);
	pop(&cartype3);
	printf("%s\n",Stack.stach);
}

しかし、コンパイル・実行は失敗し、代わりにエラーが出てきました。エラー箇所を示すバッテンマークは、void push(Stack *stack){...で始まる行とvoid pop(Stack *stack){...で始まる行の2カ所の隣にそれぞれついていました。
*や->の使い分け等にも手ごたえを感じていて、構造体を使う授業では関数型がvoid型のプログラムを多く見てきたのもあり、その類のプログラムには自信がありました。それゆえに今回、作りたい関数をint型からvoid型に変えてみただけのはずが何故エラーになってしまったのか分からず、困っています。
間違い部分を訂正しつつ、より良いプログラムにしたいです。アドバイスお待ちしています。

0

スタック領域はグローバル変数にあるので、関数に渡す必要はないですよね?
最大いくつスタックできるようにしますか?
その数分のスタック領域が必要ではありませんか?

pushする値を関数の引数で渡すのではありませんか?
引数に渡すのは文字列(charの配列)ではありませんか?
渡された文字列をスタック領域にコピーしないといけないのではありませんか?

popした値は引数で指定された文字列領域にコピーして返すのではありませんか?
引数に渡すのは文字列(charの配列)領域ではありませんか?
スタックにある文字列を引数で指定された領域にコピーしないといけないのではありませんか?

1Like

コードの掲載時のミスかと思うのですが、コンパイルが通りらないので次のように修正しました.

 void push(Stack *stack){
+void push(carEtc *stack){
 	int input;
 	if (stack->size >= N) {
 		printf("failed\n");
 	}
 	stack->stach[stack->size] = input;
 	stack->size++;
 }
 
-void pop(Stack *stack){
+void pop(carEtc *stack){

が、push() の引数が誤りのような気がします.

push() に渡したいパラメータは、cartype1 〜 cartype4 なのではないでしょうか?.
つまり void push(char * p) になるかと思います.

0Like

可能なかぎり元コードを使うと、次のような感じになりました.
pop() だけは、引数は不要、且つ、戻り値で返す方が良い、と思ったので変更しました.

下記コードを実行すると次のような結果になります.

実行結果

$ ./a.out 
[push] Stack[0] にデータ(自動車)を保存しました
[push] データ(原動機付自転車)の長さは 21 であり、N(10)を超過したため、保存しません
[push] Stack[1] にデータ(軽車両)を保存しました
[push] データ(路面電車)の長さは 12 であり、N(10)を超過したため、保存しません
[pop] Index => 1
pop => 軽車両
[pop] Index => 0
pop => 自動車

コード

#include <stdio.h>
#define N 10

typedef struct vehicle{
  char stach[N];
}carEtc;

carEtc Stack[4] = { '\0' }; // スタックに積めるデータの個数は 4個まで
int Index = 0; // スタックの位置を指す変数. 今回のスタックは4個まで積めるので、0〜3 の範囲.

void push(char * p){
  int len = strlen(p); // 入力文字列の長さを得る. 例: 「自動車」なら 9 である.
  if (len >= N) {
    printf("[push] データ(%s)の長さは %d であり、N(%d)を超過したため、保存しません\n", p, len, N);
    return;
  }
  memcpy(&Stack[Index], p, len); // p のポイント先はスタック領域なので、データは Deep Copy する
  printf("[push] Stack[%d] にデータ(%s)を保存しました\n", Index, p);
  Index++;
}

char * pop(void){
  char * p = NULL;
  if(Index > 0){ // スタックにデータが存在しているか?
    Index--;
    printf("[pop] Index => %d\n", Index);
    p = Stack[Index].stach; // Stack[Index].stach はグローバル変数なので Shallow Copy で良い
  }
  return p;
}

int main(void){
  char cartype1[] = "自動車";
  char cartype2[] = "原動機付自転車";
  char cartype3[] = "軽車両";
  char cartype4[] = "路面電車";
  push(&cartype1[0]);
  push(&cartype2[0]);
  push(&cartype3[0]);
  push(&cartype4[0]);
  printf("pop => %s\n", pop());
  printf("pop => %s\n", pop());
  printf("pop => %s\n", pop());
  printf("pop => %s\n", pop());
  return 0;
}

0Like

構造体を渡すようにすれば楽できそうですね。
popもvoid型にしてみました。

#include <stdio.h>

#define NAME_MAX (32)
#define STACK_MAX (3)

typedef struct {
    char name[NAME_MAX];
} viecle_t;

struct {
    viecle_t data[STACK_MAX];
    int index;
} stack = { .index = 0 };

void push(viecle_t *viecle) {
    if (stack.index >= STACK_MAX) {
        printf("[push] スタックが一杯です\n");
        return;
    }
    stack.data[stack.index] = *viecle;
    printf("[push] stack.data[%d] にデータ(%s)を保存しました\n",
           stack.index, viecle->name);
    stack.index++;
}

void pop(viecle_t *viecle) {
    if (stack.index <= 0) {
        printf("[pop] スタックにデータがありません\n");
        static const viecle_t EMPTY = {""};
        *viecle = EMPTY;
        return;
    }
    stack.index--;
    printf("[pop] stack.index => %d\n", stack.index);
    *viecle = stack.data[stack.index];
}

int main(void){
    viecle_t viecle1 = {"自動車"};
    viecle_t viecle2 = {"原動機付自転車"};
    viecle_t viecle3 = {"軽車両"};
    viecle_t viecle4 = {"路面電車"};
    viecle_t viecle;
    push(&viecle1);
    push(&viecle2);
    push(&viecle3);
    push(&viecle4);
    pop(&viecle); printf("pop => %s\n", viecle.name);
    pop(&viecle); printf("pop => %s\n", viecle.name);
    pop(&viecle); printf("pop => %s\n", viecle.name);
    pop(&viecle); printf("pop => %s\n", viecle.name);
    return 0;
}
実行結果
[push] stack[0] にデータ(自動車)を保存しました
[push] stack[1] にデータ(原動機付自転車)を保存しました
[push] stack[2] にデータ(軽車両)を保存しました
[push] スタックが一杯です
[pop] stack_index => 2
pop => 軽車両
[pop] stack_index => 1
pop => 原動機付自転車
[pop] stack_index => 0
pop => 自動車
[pop] スタックが空です
pop =>
1Like

@robozushi10さん、@shiracamusさん、ご回答ありがとうございます。
@shiracamusさんのプログラムのvoid push(const viecle_t *viecle) {の部分について、これをconstの文字を抜いたvoid push(viecle_t *viecle) {とするのもアリでしょうか?
@robozushi10さんと@shiracamusさんのプログラムを見ていて、「pop関数に引数が要らない理由は、原則スタックの種類に関わらず最後部にあるものから逐次popしていくため、取り出すスタックを引数等で区別する必要は特にないからではないか」と思うようになりました。これは正しい見解でしょうか?

0Like

➀ありですけど、渡された構造体の内容を変更しないということを明示するためにconstを指定しています。
constがないと引数で渡された構造体の内容を変更されるかもしれないという不安が残ります。
呼び出す側はconstでなくても大丈夫です。
例えば、strlen関数も、引数の文字列用域を変更しないのでconstが付いています。

②スタックから取り出したデータはどこに格納しますか?
引数で指定された領域に格納するか、戻り値で返すか、どちらかを選ぶことになります。
関数をvoid型にするのであれば、データ格納用の引数が必要です。
戻り値で返すのであれば引数は不要ですが、void型以外にしないといけません。

0Like

②について
はい、スタックは常に先頭位置を返すので、引数不要です。

なお、戻り値については、popという関数名は「取り出す」という意味合いになり、「引数で返す」方が自然かと思い、(引数で渡されたポインタに詰めないように)変更しました

0Like

私のコメントに書いたコードですが、データ構造 と スタック構造 が区別できるように、スタック関連変数 を stack構造体 に変更しました。

0Like

@shiracamusさん、@robozushi10さん、詳しい説明ありがとうございます。
constは絶対に必要なわけではなく、しかし構造体の構成要素に変更を入れない場合にユーザ側へそれを保証するのに ”あると良い” もの、といった感じですね?
また、popという関数名が「取り出す」という意味を持つ点についても納得出来ました。

♢♢♢

ちなみに、元の問題の自分の解答(pushとpopがint型)については、まず最初に答えたプログラムでエラー判定されました。その後以下のように書き直し、今回学んだ内容や模範解答になるべく近づけてはみたものの、これもエラー判定でした。

#include <stdio.h>
#define N 10

typedef char *data_t;
data_t stack[N];
int size;

void init(void){
	size = 0;
}

int push(data_t *x){
	if (size >= N) {
		return 1;
	}
	stack[size] = *x;
	size++;
	return 0;
}

int pop(data_t *x){
	if (size <= 0) {
		return 1;
	}
	*x = stack[size - 1];
	size--;
	return 0;
}

int main(void){
	char item1[] = "剣";
	char item2[] = "盾";
	char item3[] = "薬草";
	char item4[] = "ルビー";
	push(&item1);
	push(&item2);
	push(&item3);
	push(&item4);
	char *p;
	for(int i=0;i<4;i++){
		pop(&p);
		printf("%s\n", p);
	}
}

以下、期待していた実行結果です。

ルビー
薬草
盾
剣

ここで、出来れば元の問題のプログラム(前述の、pushやpopがint型のバージョン)についても一緒に考えてくださると幸いです。些細な内容でも良いので、ご教授お願いします。

0Like

int push(data_t *x){ の引数は char **x と同等ですが、呼び出す側で配列名に&を付けても単なるchar *でしかないので型不整合エラーです。
int push(data_t x){ にしてchar *を引数にするといいです。
そして、char *data_t に置き換えてしまいましょう。
それと、init()を呼ぶのを忘れてます。

#include <stdio.h>
#define N 10

typedef char* data_t;
data_t stack[N];
int size;

void init(void){
	size = 0;
}

int push(data_t x){
	if (size >= N) {
		return 1;
	}
	stack[size] = x;
	size++;
	return 0;
}

int pop(data_t *p){
	if (size <= 0) {
		return 1;
	}
	*p = stack[size - 1];
	size--;
	return 0;
}

int main(void){
	init();
	data_t item1 = "剣";
	data_t item2 = "盾";
	data_t item3 = "薬草";
	data_t item4 = "ルビー";
	push(item1);
	push(item2);
	push(item3);
	push(item4);
	for(int i=0;i<4;i++){
		data_t item;
		pop(&item);
		printf("%s\n", item);
	}
}
2Like

@shiracamusさん

int push(data_t *x){の引数はchar **xと同等ですが、呼び出す側で配列名に&を付けても単なるchar *でしかないので型不整合エラーです。

なるほどですね!
すると、「int pop(data_t *p){の引数もchar **pと同等で、しかしmain関数側での型不整合エラーにはならない」といった感じになってしまうのではないでしょうか?配列名に&を付けるとchar *になる点を考えたら、引数がchar **と同等だとポインタの数的に異なってしまうのではないかと疑問に思いました。

0Like

Your answer might help someone💌