2016-06-21 7 views
2

スタックを考えてみましょう。スタックの最大値は100です。私はこのポップ機能していC.スタックスタックC.ポップアリトップ結果の保存

#define MAX 100 

typedef struct stack { 
    int size; 
    int values[MAX];  
} STACK; 

:として定義され、

int pop(STACK *s, int *x){ 
    if (s->size == 0) return 1; 

    int *p = s->values + s->size--; 
    *x = *p; 

    return 0; 
} 

値を削除することになっている[MAX]最後の要素をXアドレスにその値を格納し、成功ならば0を返します。

その他の機能:

int top(STACK *s, int *x){ 
    if (s->size == 0) return 1; 

    int *p = s->values + s->size; 
    *x = *p; 

    return 0; 
} //like pop function, it should store top element at address x. 


void initStack(STACK *s){ 
    s->size = 0; 
} 

int push(STACK *s, int x){ 
    if (s->size >= MAX) return 1; 

    int *p = (s->values); 
    *(p + s->size++) = x; 

    return 0; 
} 

これが私のメインです。

int main(){ 
    struct stack s; 
    STACK *p = &s; 
    int i; 
    int x,y,z,w,t; 

    initStack(p); 

    for(i = 0; i < MAX; i++) 
     push(p,i); 

    int res = push(p,MAX); 

    for(i = 0; i < MAX; i++) 
     printf("%d|", p->values[i]); 

    printf("\nLast insertion: %d",res); 

    pop(p,&x); 
    pop(p,&y); 
    pop(p,&z); 
    pop(p,&w); 
    top(p,&t); 

    printf("\nThe elements %d|%d|%d|%d were removed. Current stack size: %d . Top element: %d.",x,y,z,w,p->size,t); 

    return 0; 
} 

結果(最後のprintf):何らかの理由で

The elements 1|99|98|97 were removed.Current stack size: 96 .Top element: 96. 

、最初のポップ呼び出しが失敗し、だけでなく、取り除かれた要素のリストを非難するが、それだけで最初のポップ呼び出しに失敗しましたまた、最上位の要素の結果。理由についてのご提案は ですか?

+0

何push' 'については? –

+4

そして、これをしないでください: 'int * p = s-> values + s-> size - ' please。 –

+0

スタックを完全に埋める場合にのみ問題が発生しますか? ( 'push 'と' initStack'も投稿してください) – molbdnilo

答えて

0

私はコードの明瞭度についてコメントした人です。配列を使用している場合は、ポインタ演算の代わりに[]のインデックスを使用すると、問題が発生する可能性は非常に低くなります。

つまり、私は問題はpopには範囲外の問題があると言います。 mainMAX要素をプッシュすると、s->size==MAXになります。したがって、pop*(s->values + MAX) == s->values[MAX]にアクセスします。実際には配列の末尾にあるの後にはの要素です。配列は0からMAX-1までで、1からMAXまでではありません。たとえば、size==1の場合、実際にはs->values[1]ではなく、最初の要素であるs->values[0]が必要です。

最小変更はint *p = s->values + --s->size;となりますが、問題を解決する正しい方法ではないことに同意します。代わりに、ループがMAX要素をプッシュし、ループ後の1つの以上pushのコールがあるので、あなたがスタックにMAX+1要素をプッシュする*x = s->values[s->size-1]; --s->size;

+0

なぜ2番目のソリューションが最初のソリューションより優れていますか? –

+0

私の意見:(1)配列のインデックス演算子 '[]'を使うため、ポインタ操作を使用するよりも明瞭です。 (2)インデックスをデクリメントから分離します。これも明確です(デバッガではシングルステップが簡単です)。 (3)コードを実際にやりたいことともっと明確に関連付ける:アイテムを取得し、サイズを変更する( ' - ')。 – cxw

2
  1. のようなものを使用。
  2. あなたpop実装があるためラインの誤りです。

    int *p = s->values + s->size--; 
    

    s->sizeの減少がpへの評価と代入した後に発生しますので、s->values[s->size]代わりのs->values[s->size-1]を指しますpポインタ。

  3. 式内で増分/減算演算子を使用することは正当な悪い習慣とみなされ、これらの種類のバグのために厳密に推奨されません。

  4. 可能、それはそうのようなインデックス演算子[]代わりのポインタ演算を使用することがはるかに読みやすく、よりエラーを起こしやすいです。

    int 
    pop(STACK *s, int *x) { 
        if (s->size == 0) { 
        return 1; 
        } 
    
        s->size--; 
        *x = s->values[s->size]; 
    
        return 0; 
    } 
    
+0

最後のプッシュはエラーケースをテストしているようですので、ここに目的があります。 – ElderBug

+1

はい、私はそれが事実かもしれないと思ったが、そうでなければこれを指摘したかった。 – s7amuser