2012-01-23 20 views
0

関数からポインタを返すと、その値に個別にアクセスできます。しかし、そのポインタ変数の値を出力するためにループが使用されると、間違った値が表示されます。私が間違っているところでは、それを理解することはできません。C++の関数からポインタを返す

#include <iostream> 
#include <conio.h> 

int *cal(int *, int*); 

using namespace std; 

int main() 
{ 
    int a[]={5,6,7,8,9}; 
    int b[]={0,3,5,2,1}; 
    int *c; 
    c=cal(a,b); 

    //Wrong outpur here 
    /*for(int i=0;i<5;i++) 
    { 
     cout<<*(c+i); 
    }*/ 

    //Correct output here 
    cout<<*(c+0); 
    cout<<*(c+1); 
    cout<<*(c+2); 
    cout<<*(c+3); 
    cout<<*(c+4); 

return 0; 
} 

int *cal(int *d, int *e) 
{ 
    int k[5]; 
    for(int j=0;j<5;j++) 
    { 
     *(k+j)=*(d+j)-*(e+j); 
    } 
    return k; 
} 
+10

コンパイラの警告を有効にしてから、それらを読み取ります。 –

+1

関数からポインタを返す必要があるケースはごくわずかです。私の経験上、ポインターを返す必要性は、欠陥のあるプログラムの設計に起因することが最も多いです。ほとんどの場合、結果をパラメータの1つに戻して、呼び出し側がデータをどこに割り当てるか心配する必要があります。 – Lundin

+2

@ Lundinいくつかの注目すべき例外(たとえば、失敗する可能性のある検索機能)があります。一方、パラメータの1つを返すことは、プロファイラがあなたに選択肢がないと言ったときに、絶対に必要なときにのみ使用するべきです。ほとんどの場合、正しい解決策は価値によって戻ることです。もちろん、Cスタイルの配列を使用しないことを意味します(しかし、これは一般的にお勧めです)。 –

答えて

1

int k[5]配列をスタック上に作成されます。だから、calから戻って範囲外になると破壊されます。あなたは、出力配列として三番目のパラメータを使用することができます。

void cal(int *d, int *e, int* k) 
{ 
    for(int j=0;j<5;j++) 
    { 
     *(k+j)=*(d+j)-*(e+j); 
    } 
} 

このような呼び出しcal

int a[]={5,6,7,8,9}; 
int b[]={0,3,5,2,1}; 
int c[5]; 
cal (a, b, c); // after returning from cal, c will be populated with desired values 
+0

あなたは 'void'リターン関数で値を返しています! :) –

+0

@ another.anon.cowardが修正されました – Meysam

+0

あなたは未処理のポインタで遊んでいますが、その必要はありません! – Johnsyweb

6

ポインタをローカル変数に戻しています。

kがスタック上に作成されます。 cal()がスタックを終了するとメモリは解放されます。後でそのメモリを参照すると、未定義の動作になります(ここでは美しく説明されています:https://stackoverflow.com/a/6445794/78845)。

あなたのC++コンパイラはこれについて警告し、これらの警告に注意する必要があります。何が価値があるために

は、ここで私はC++でこれを実装したい方法です:

#include <algorithm> 
#include <functional> 
#include <iostream> 
#include <iterator> 

int main() 
{ 
    int a[] = {5, 6, 7, 8, 9}; 
    int b[] = {0, 3, 5, 2, 1}; 
    int c[5]; 
    std::transform (a, a + 5, b, c, std::minus<int>()); 
    std::copy(c, c + 5, std::ostream_iterator<int>(std::cout, ", ")); 
} 

See it run!

+3

int k [5]が破棄されます。 – RvdK

+0

@PoweRoy:「破壊される」べきではないか?破壊されることは保証されていませんか?それは確かに違法なアクセスです... –

+1

@ another.anon.coward:それは破壊されます、そうでなければRAIIは動作しません。他のものがそのメモリを使用するかどうかは保証できません。 – Johnsyweb

0

他の人が指摘したように、あなたがあるローカル 変数へのポインタを返すしています未定義の動作。しかし、実際の問題は で、配列を返す必要があり、Cスタイルの配列は壊れています。 配列をstd::vector<int>に置き換え、ポインタを忘れて (値を扱っているため)、コードが機能します。

+0

'std :: vector'(またはvalarray)は実際にはここでは必須ではありませんが、あなたが正しいので、ポインタは必要ありません。 – Johnsyweb

+0

@Johnsyweb彼の特定の例では、 'std :: vector 'は明らかで正しい解です。 –

+0

C++ 11のinitialserリストで私はあなたに同意するかもしれませんが、 "正しい"は主観的です。私は、たとえば、[私の答え](http://stackoverflow.com/a/8969311/78845)で提供されているコードが "正しい" mutは標準ライブラリのコンテナを使用していないと考えています。 – Johnsyweb

関連する問題