[C++] Een array passen aan een functie lukt niet

[Arcane Apex]

Ik probeer een tweedimensionale array te passen aan een functie en deze te vullen met getallen, maar tijdens compilatie geeft g++ de volgende warnings en errors:

In function 'int main()':
11: warning: array subscript is above array bounds
11: warning: array subscript is above array bounds
(.text+0x6c): undefined reference to `maak2dArray(int)'
collect2: ld returned 1 exit status

Het lukt me echter niet om de code compilerend te krijgen. Het gaat om de volgende code.
Weet iemand welke aanpassingen er aan de code gemaakt moeten worden, zodat deze zonder warnings en errors compileert?

code:

using namespace std;

const unsigned int XGROOTTE = 4; 
const unsigned int YGROOTTE = 4;

void maak2dArray(int);

int main()
{   
    int mijn2dArray[XGROOTTE][YGROOTTE]={{0}};
    maak2dArray(mijn2dArray[XGROOTTE][YGROOTTE]);

    return 0;
}

void maak2dArray(int mijn2dArray[XGROOTTE][YGROOTTE]){  
            
    int aantalVakjes = XGROOTTE * YGROOTTE;
    
    //Maak een array van variabele grootte
    int *inhoudArray; 
    inhoudArray = new int[aantalVakjes];
    
    
    //Maak een lijst met inhoud(getallen)
    for(int i=0; i<=aantalVakjes; i++){ 
        inhoudArray[i] = i; //Vul de inhoudArray met getallen.
    }
    
    //Vul mijn2dArray met de lijst met getallen
    for(unsigned int y=0; y<=YGROOTTE; y++){
    
        int i = 0;
        
        for(unsigned int x=0; x<=XGROOTTE; x++){ 
            mijn2dArray[x][y]=inhoudArray[i];
            i++;
        }       
    }
}

[KopjeThee]

Sowieso deze aanpassing: Overal waar je in je for-loops <= hebt staan, moet dat < worden.

[Verwijderd]

Er zijn dan nog 3 fouten:

• De functiedefinitie van maak2dArray verschilt met de declaratie ervan.
  Het moet 'void maak2dArray(int[XGROOTTE][YGROOTTE])' zijn ipv 'void maak2dArray(int)'.
• maak2dArray(mijn2dArray) ipv maak2dArray(mijn2dArray[XGROOTTE][YGROOTTE])
• Je hebt een memory leak in maak2dArray, je vergeet namelijk inhoudArray te deleten

[ Voor 3% gewijzigd door Verwijderd op 07-02-2010 08:40 ]

[Arcane Apex]

Ok, ik heb alle aanbevelingen doorgevoerd en de code compileert nu zonder fouten.
Nu is er echter een ander probleem. Ik heb de code iets uitgebreid om de inhoud van de array op het scherm te printen. Echter in plaats van getallen op het scherm geprint te krijgen, krijg ik smileys op het scherm geprint.
Dat is uiteraard niet de bedoeling. Heeft iemand enig idee waarom dat gebeurt?

code:

#include <iostream>
#include <cstdlib>
#include <string>

using namespace std;

const unsigned int XGROOTTE = 4; 
const unsigned int YGROOTTE = 4;
const unsigned int AANTAL_VAKJES = XGROOTTE * YGROOTTE;

void maak2dArray(int mijnArray[XGROOTTE][YGROOTTE]);
string maakLijstVanInhoud(int mijnArray[XGROOTTE][YGROOTTE]);

int main()
{
    int mijnArray[XGROOTTE][YGROOTTE]={{0}};
    maak2dArray(mijnArray);
    
    cout << maakLijstVanInhoud(mijnArray) << endl << endl;
        

    system("pause"); 

    return 0;
}


void maak2dArray(int mijnArray[XGROOTTE][YGROOTTE]){ 
    
    for(unsigned int y=0; y<YGROOTTE; y++){
        
        int i = 0;
        
        for(unsigned int x=0; x<XGROOTTE; x++){ 
            mijnArray[x][y]=i; 
            i++;
        }       
    }   
}

string maakLijstVanInhoud(int mijnArray[XGROOTTE][YGROOTTE]){

    string mijnString;
    
    for(unsigned int y=0; y<YGROOTTE; y++){     
        
        for(unsigned int x=0; x<XGROOTTE; x++){ 
            mijnString += mijnArray[x][y];
            mijnString += ",";          
        }       
    }

    return mijnString;
}

[^Mo^]

Een std::string heeft ook geen += operator om getallen toe te voegen (of uberhaupt een manier om van een getal naar een string te gaan).

Je kan twee dingen doen (ja, vast wel meer ook..)
1) Maak er een stringstream van (naar mijn mening het netste), of...
2) Gebruik de itoa functie

[Arcane Apex]

^Mo^ schreef op zondag 07 februari 2010 @ 10:21:
Een std::string heeft ook geen += operator om getallen toe te voegen (of uberhaupt een manier om van een getal naar een string te gaan).

1) Maak er een stringstream van (naar mijn mening het netste), of...

Dat bleek inderdaad het probleem en de stringstream-oplossing deed het allemaal correct werken.

Ik heb echter nog 1 laatste vraag. Toen ik in de vorige versie van de code die memory leak probeerde op te lossen deed ik dat met:

delete [] inhoudArray;

Echter bleek dit alleen maar een warning op te leveren tijdens het compileren. Hoe kan ik het geheugen dat de array inneemt weer vrijmaken, zodat die array geen memory leak meer veroorzaakt. Het bovenstaande stukje code wat op andere forums wordt aanbevolen lijkt namelijk niet te leiden tot een waarschuwingloze compilatie. Daarnaast crasht de applicatie door deze regel tijdens runtime.

De desbetreffende waarschuwing welke de compiler geeft: warning: deleting array 'int inhoudArray [16]'

[ Voor 9% gewijzigd door Arcane Apex op 07-02-2010 10:55 ]

[Verwijderd]

Dat zou nochtans de correcte manier geweest zijn om de array te deleten. Kan je misschien anders even de foutmelding posten.

Dit compileert nochtans:

C++:

void maak2dArray(int mijn2dArray[XGROOTTE][YGROOTTE]){

    int aantalVakjes = XGROOTTE * YGROOTTE;

    //Maak een array van variabele grootte
    int *inhoudArray;
    inhoudArray = new int[aantalVakjes];


    //Maak een lijst met inhoud(getallen)
    for(int i=0; i<aantalVakjes; i++){
        inhoudArray[i] = i; //Vul de inhoudArray met getallen.
    }

    //Vul mijn2dArray met de lijst met getallen
    for(unsigned int y=0; y<YGROOTTE; y++){

        int i = 0;

        for(unsigned int x=0; x<XGROOTTE; x++){
            mijn2dArray[x][y]=inhoudArray[i];
            i++;
        }
    }
    delete []inhoudArray;
}

[ Voor 74% gewijzigd door Verwijderd op 07-02-2010 10:57 ]

[Arcane Apex]

Verwijderd schreef op zondag 07 februari 2010 @ 10:54:
Dat zou nochtans de correcte manier geweest zijn om de array te deleten. Kan je misschien anders even de foutmelding posten.

De waarschuwing welke de compiler geeft: warning: deleting array 'int inhoudArray [16]'

[Verwijderd]

De foutmelding doet mij vermoeden dat inhoudArray dan niet meer dynamisch gealloceerd was, maar iets in deze vorm.

C++:

int inhoudArray[16];

Wat natuurlijk automatisch wordt opgeruimd.

[Killemov]

Arcane Apex schreef op zondag 07 februari 2010 @ 10:45:
[...]


Dat bleek inderdaad het probleem en de stringstream-oplossing deed het allemaal correct werken.

Ik heb echter nog 1 laatste vraag. Toen ik in de vorige versie van de code die memory leak probeerde op te lossen deed ik dat met:

delete [] inhoudArray;

Echter bleek dit alleen maar een warning op te leveren tijdens het compileren. Hoe kan ik het geheugen dat de array inneemt weer vrijmaken, zodat die array geen memory leak meer veroorzaakt. Het bovenstaande stukje code wat op andere forums wordt aanbevolen lijkt namelijk niet te leiden tot een waarschuwingloze compilatie. Daarnaast crasht de applicatie door deze regel tijdens runtime.

De desbetreffende waarschuwing welke de compiler geeft: warning: deleting array 'int inhoudArray \[16]'

Delete gebruik je alleen als tegenhanger van new.

int mijn2dArray[XGROOTTE][YGROOTTE]={{0}};

Dit assignment is nogal vreemd. Je vult er alleen element [0] (of [0][0]) mee.

[Verwijderd]

Killemov schreef op zondag 07 februari 2010 @ 11:02:
[...]

Dit assignment is nogal vreemd. Je vult er alleen element \[0] (of \[0]\[0]) mee.

Niets mis met die assignment, alle niet gedefinieerde elementen krijgen dan de default waarde, dus ook gewoon 0.

[Arcane Apex]

Verwijderd schreef op zondag 07 februari 2010 @ 10:54:

Dit compileert nochtans:

C++:

void maak2dArray(int mijn2dArray[XGROOTTE][YGROOTTE]){

    int aantalVakjes = XGROOTTE * YGROOTTE;

    //Maak een array van variabele grootte
    int *inhoudArray;
    inhoudArray = new int[aantalVakjes];


    //Maak een lijst met inhoud(getallen)
    for(int i=0; i<aantalVakjes; i++){
        inhoudArray[i] = i; //Vul de inhoudArray met getallen.
    }

    //Vul mijn2dArray met de lijst met getallen
    for(unsigned int y=0; y<YGROOTTE; y++){

        int i = 0;

        for(unsigned int x=0; x<XGROOTTE; x++){
            mijn2dArray[x][y]=inhoudArray[i];
            i++;
        }
    }
    delete []inhoudArray;
}

Dit was inderdaad de oplossing voor het memory leak probleem. Iedereen in deze thread; heel erg bedankt. Alles werkt nu soepel. Thanks!

[Stukfruit]

Ts, er zitten nog steeds veel fouten en onjuiste programmeertechnieken in.

• Het op 0 zetten van iedere waarde in zo'n buffer als deze doe je in deze situatie niet met "= { { 0 } };", maar met memset(&mijn2dArray, 0, sizeof(int)*XGROOTTE*YGROOTTE);.

• Het doorgeven van een multi-dimensionale array als argument aan een functie is (voor zover ik weet) een GCC-extensie. Met andere compilers werkt dit niet, tenzij je gaat lopen casten en er hele vieze code van maakt. Als je nog eens Windows-programma's wil gaan maken met MSVC, dan kan je jezelf beter aanleren om hier geen gebruik van te maken.

• Iedere keer dat je geheugen aanvraagt met new en verwijdert met delete zorg je voor fragmentatie van het geheugen. Na een paar keer is dit niet zo erg, maar als je dit (erg) vaak doet dan krijg je Firefox-achtige problemen (je programma lijkt een hoop geheugen nodig te hebben terwijl het weinig echt gebruikt). In jouw code is het (imho) overbodig misbruik van deze functies.

• Het heen en weer gooien van data op deze manier is erg vies en bij intensief gebruik ervan ook langzamer. Je zou het beter kunnen oplossen door meteen die buffer met new aan te maken en die dan door te geven aan de functie die de data invult. Vertel hierbij ook altijd aan de functie hoe groot de gegeven buffer is, om bugs te voorkomen.

• Een 2d-waarde in een eendimensionale buffer kan je ook aanspreken door de volgende berekening te gebruiken: y * XGROOTTE + x

.

C++:

#include <string>
#include <iostream>

using namespace std;

const int XGROOTTE = 4; 
const int YGROOTTE = 4;
const int AANTVAKJES = XGROOTTE * YGROOTTE;


// maak2dArray doet nu alleen nog aan vakkenvullen, verder niets
void maak2dArray(int* mijn2dArray, int hoelang);
void maak2dArray(int* mijn2dArray, int breedte, int hoogte);
void schrijfArray(int* mijn2dArray, int hoelang);


int main(int argc, char* argv[])
{    
    // memset is hier eigenlijk niet nodig omdat je de array toch al meteen hierna invult
    int* mijn2dArray = new int[AANTVAKJES];
    memset(mijn2dArray, 0, sizeof(int)*AANTVAKJES);
    schrijfArray(mijn2dArray, AANTVAKJES);

    // Als je nu bv pixels in een buffer wil schrijven, dan doe je dit..
    maak2dArray(mijn2dArray, XGROOTTE, YGROOTTE);
    schrijfArray(mijn2dArray, AANTVAKJES);

    // ..maar in dit geval kan het ook zo!
    maak2dArray(mijn2dArray, AANTVAKJES);
    schrijfArray(mijn2dArray, AANTVAKJES);

    delete [] mijn2dArray;
    return 0;
}


// Oplossing 1
void maak2dArray(int* mijn2dArray, int hoelang)
{
    for (int i=0; i<hoelang; i++)
        mijn2dArray[i] = i;
}


// Oplossing 2
void maak2dArray(int* mijn2dArray, int breedte, int hoogte)
{
    int i = 0;

    for (int y=0; y<hoogte; y++)
        for (int x=0; x<breedte; x++)
            mijn2dArray[y * breedte + x] = i++;
}


void schrijfArray(int* mijn2dArray, int hoelang)
{
    for (int i=0; i<hoelang; i++)
        cout << mijn2dArray[i] << ", ";

    cout << endl;
}

En het ziet er ook simpeler uit dan al die ingewikkelde constructies

[Verwijderd]

een mogelijke oplossing zou kunnen zijn je 'array' te wrappen in een struct...

code:

struct MyArrayType
{
   int vakjes[X_SIZE][Y_SIZE];
};

en dan dus deze struct als argument meegeven...
maakt ook het 'new-en' en 'delete-en' van je array wat makkelijker, want dat wordt nu gewoon:

MyArrayType* foo = new MyArrayType;

en

delete foo;

anyway, tis maar een gedachte....

[.oisyn]

[quote]Stukfruit schreef op zondag 07 februari 2010 @ 18:00:
Ts, er zitten nog steeds veel fouten en onjuiste programmeertechnieken in.

• Het op 0 zetten van iedere waarde in zo'n buffer als deze doe je in deze situatie niet met "= { { 0 } };", maar met memset(&mijn2dArray, 0, sizeof(int)*XGROOTTE*YGROOTTE);.
Onzin, je kunt een aggregate gewoon zero-initializen met = { }. Voor een POD array als deze zal de compiler doorgaans zelf een call naar memset genereren. Maar wat met memset() niet kan en met = { } wel is aggregates met members met constructors zero-initializen.

[q]• Het doorgeven van een multi-dimensionale array als argument aan een functie is (voor zover ik weet) een GCC-extensie. Met andere compilers werkt dit niet, tenzij je gaat lopen casten en er hele vieze code van maakt. Als je nog eens Windows-programma's wil gaan maken met MSVC, dan kan je jezelf beter aanleren om hier geen gebruik van te maken.
Eveneens onzin, de signature van foo(int[X][Y]) is in werkelijkheid foo(int(*)[Y]), en die kun je dus elke tweedimensionale array geven waarbij de achterste dimensie Y groot is. De code zoals voorgesteld is gewoon ISO C++ compliant.

[q]• Iedere keer dat je geheugen aanvraagt met new en verwijdert met delete zorg je voor fragmentatie van het geheugen. Na een paar keer is dit niet zo erg, maar als je dit (erg) vaak doet dan krijg je Firefox-achtige problemen (je programma lijkt een hoop geheugen nodig te hebben terwijl het weinig echt gebruikt). In jouw code is het (imho) overbodig misbruik van deze functies.
Dit is echt veel te kort door de bocht. In de regel is je geheugen niet meer gefragmenteerd als je eerst X allocaties doet en ze dan alle X weer vrijgeeft. Hier gebeuren geen andere allocaties tussen de new en de delete (even ervan uitgegaan dat het een singlethreaded applicatie is) en dus zal fragmentatie geen issue zijn.

[q]• Het heen en weer gooien van data op deze manier is erg vies en bij intensief gebruik ervan ook langzamer. Je zou het beter kunnen oplossen door meteen die buffer met new aan te maken en die dan door te geven aan de functie die de data invult. Vertel hierbij ook altijd aan de functie hoe groot de gegeven buffer is, om bugs te voorkomen.
Beter wrap je het gewoon in een class ipv zelf handmatig aan te moeten klooien met pointers en buffergroottes. Maar hey wacht, die bestaat al: std::vector

[q]• Een 2d-waarde in een eendimensionale buffer kan je ook aanspreken door de volgende berekening te gebruiken: y * XGROOTTE + x
Een goede manier om je code minder leesbaar te maken idd, chapeau!

Al met al vind ik de door jou aangedragen punten wat overdreven.

[ Voor 3% gewijzigd door .oisyn op 07-02-2010 23:41 ]

[Stukfruit]

.oisyn schreef op zondag 07 februari 2010 @ 23:34:
Onzin, je kunt een aggregate gewoon zero-initializen met = { }. Voor een POD array als deze zal de compiler doorgaans zelf een call naar memset genereren. Maar wat met memset() niet kan en met = { } wel is aggregates met members met constructors zero-initializen.

{ } kan het ook? Oke, dat wist ik niet. I stand corrected dan

Eveneens onzin, de signature van foo(int[X][Y]) is in werkelijkheid foo(int(*)[Y]), en die kun je dus elke tweedimensionale array geven waarbij de achterste dimensie Y groot is. De code zoals voorgesteld is gewoon ISO C++ compliant.

Hm, net even geprobeerd en het werkt idd.

Hier kan ik verder niets anders op zeggen dan dat ik zeker weet dat er wel een probleem mee was, maar ik ben even helemaal kwijt hoe en wat. Ik weet in ieder geval dat ik eens heb lopen prutsen met de code van iemand die wel met GCC werkte maar *niet* met MSVC, op geen enkele manier (en dat had dus met die multidimensionale arrays als argument te maken).

Ik zal eens kijken of ik er nog iets over kan vinden, ik haal vast weer dingen door elkaar

Dit is echt veel te kort door de bocht. In de regel is je geheugen niet meer gefragmenteerd als je eerst X allocaties doet en ze dan alle X weer vrijgeeft. Hier gebeuren geen andere allocaties tussen de new en de delete (even ervan uitgegaan dat het een singlethreaded applicatie is) en dus zal fragmentatie geen issue zijn.

Dat probeerde ik dus te zeggen (maar wel een beetje onduidelijk ja). Als hij al voor zulke dingen snel wat allocaties doet dan durf ik te voorspellen dat hij dat later in grotere programma's ook (door elkaar) gaat doen. Hij zal niet de eerste zijn die dit doet.

Beter wrap je het gewoon in een class ipv zelf handmatig aan te moeten klooien met pointers en buffergroottes. Maar hey wacht, die bestaat al: std::vector

Zo te zien leert hij en van std::vector leer je weinig.

Een goede manier om je code minder leesbaar te maken idd, chapeau!

Dat soort code stop je later toch in een leuke class met grappige aanhangmethodes, waarna je er nooit meer wat van ziet. Zie verder m'n punt wbt std::vector hierboven.