[c++] file inlezen gaat fout.

[dexter07051982]

ik probeer op de volgende manier een txt-file in te lezen.

code:

while(!readFile.eof())
{
    char* nat = new char();     // anders wordt nat in iedere Schaatser hetzelfde
    char* naam = new char();    // idem maar dan voor naam

    readFile >> naam >> nat >> uren >> seperator >> minuten >> seperator >> seconden >> seperator >> milis;
    cout << naam << "   " << nat << endl;

    Time ritTijd(uren,minuten,seconden,milis);
        
    uitslag.add(*(new Schaatser(naam, nat, ritTijd)));
}

Dit gaat voor de eerste 3 regels wel goed maar daarna niet meer.
als ik de ingelezen gegevens "cout <<" dan geeft ie dus de eerste 3 regels wel weer. daarnaa geeft ie steeds "==" gevolgd door een aantal kwadraatjes (stuk of 6).

iemand een idee wat er fout gaat.

bij de eerste file (invoer500.txt) gaat ie wel goed. bij de tweede doet ie het dus fout. (invoer5000.txt). ze gebruiken bijde dezelfde code (die hierboven staat) en de files hebben dezelfde indeling.

[AaroN]

*knip*, stond bl@@t

[ Voor 93% gewijzigd door AaroN op 12-02-2003 17:00 ]

[dexter07051982]

maar hij mag ze niet weer weggooien.
er staat namelijk een verwijzing naar in een andere class
(Schaatser)

en 2 maal 24 is toch niet een heleboel?

[ Voor 18% gewijzigd door dexter07051982 op 10-02-2003 19:11 ]

[hobbit_be]

ahem - niet echt nette code een new voor een char? laat die alvast achterwege en gebruik buffers - dan doe je een copy van die strings in je constructor van Schaatser. en een derefercing van die Schaatser? wat wil je daar mee aanvangen?. nou goed dat zijn details.

het zou best kunnen dat je je file ofwel met /r/c or /c hebt 'beeindig' maw je endofline. (open in notepad en wordpad en kijk of er een verschil is

wat ook zou kunnen je dat je new char geen 0 zet en dus niet echt een string is. C++ gaat er niet vanuit dat je memory of 0 zet dus steeds nat[0] = 0; //etc etc...

maar ik vermoed gewoon een fout in je file. doe een sprintf om te zien wat ie doet - is wel geen C++ maar nog wel sneller.

ik hoop voor jouw dat je RitTijd in de constructor niet als reference wordt gegeven... als je toch new's gebruikt kun je evengoed Time* = new Time()... dan gaat het ook allemaal wat sneller en is toch wat logischerals je die doorgeeft en maakt je code ook wat future-proof.

[dexter07051982]

in de files kon ik geen enkel verschil vinden.
en hoe werkt dat sprintf en wat heb ik er aan?
ik gebruik om alles op het scherm te zetten al "cout <<".

[hobbit_be]

sprintf is typich C maar kun je checken of het aan CIN ligt of niet. dat moet sscanf zijn niet sprintf eigenlijk ...

[MSalters]

De standaard read lus is

code:

while ( readFile >> var1 >> var 2 )
{
   gebruik(var1,en,var2);
}

eof() geeft alleen true terug als er al een EOF gezien is. Na je laatste regel is dat natuurlijk nog niet meteen het geval, maar toch probeer je dan readFile >> naam zonder te checken of dat lukt.

Evengoed ben je hier overbodig met pointers bezig. Je verliest elke lus 3 objecten. Je commentaar achter nat klopt ook niet; een variabele in een lus wordt elke keer opnieuw aangemaakt. Zelfs als dat niet het geval is zal de cout>> 'm elke keer overschrijven.

[Soultaker]

Ben ik nu de enige die ziet dat je met "new char()" een enkele character alloceert, en dat je daar dus geen string in kwijt kunt?!! Zinloos commentaar zoals dat van AaroN (en de meeste opmerkingen van hobbit_be, trouwens) kan ook wel achterwege blijven; als je geen C++ kunt programmeren, reageer dan gewoon niet.

Dat geldt trouwens ook voor de C++ code van de TS: als je niet met C-style strings (character pointers) kunt werken, gebruik dan C++ strings, die zijn wél veilig! Als je je invoer inleest zoals MSalters dat aangeeft, kan het bijna niet fout gaan.

Zoiets dus:

C++:

std::string naam, nat, whatever;
while(stdin >> naam >> nat >> whatever)
{
    // etc.
}

Het is dan verder niet nodig om zelf geheugen te alloceren; je string wordt automatisch gekopieerd waar nodig (tenzij je dat verpest door met references te werken, maar bij gebrek aan verdere code kan ik dat natuurlijk niet beoordelen).

[hobbit_be]

wow had effe daar mooi overgelezen ... dat komt ervan met java te moeten werken. Dat ie geen Exceptions gooit vind ik wel niet mooi.

maar voor de rest van mijn comments blijf ik wel bij - std::string is natuurlijk mooier C++ , no question about it, maar zo kun je natuurlijk wel doorblijven. (kun je dus NOG beter boost strings pakken . trouwens streams worden 'eigenlijk' afgeraden(van Bjorne himself, en ook Effective C++) vanwege het dubieuze order van executie die niet op elke compiler hetzelfde. doet. soit - beetje geschrokken van Soultakers comments en van mijn eigen stommigheid met de new...

[Verwijderd]

Okee, nog even recapituleren :

- Als je geheugen gaat alloc'en ('new'), zorg dan dat je weet hoeveel geheugen je wilt hebben.
(new char() alloc'ed 1 char, new char[255] alloc'ed 255 bytes...)

- als je geheugen gealloc'ed hebt, moet je 't ook weer vrijgeven.
('delete nat;' resp. 'delete [] nat;'.

- Als je dingen aan arrays / lists / whatever gaat toevoegen, ga dan geen reference geven naar iets wat je gealloceerd hebt want dat gaat geheid fout. (Wie gaat het dan weer deleten..? en wanneer..?)

en uhhh... Als je gaat programmeren, lees de neukende handleiding ('void RTFM();').
(Geintje, geen behoefte aan nog een ban - maar toch, lees de helpfiles over memory-allocation en je was al een heel eind verder geweest !)

[MSalters]

hobbit_be schreef op 11 February 2003 @ 01:51:
trouwens streams worden 'eigenlijk' afgeraden(van Bjorne himself, en ook Effective C++) vanwege het dubieuze order van executie die niet op elke compiler hetzelfde. doet. soit - beetje geschrokken van Soultakers comments en van mijn eigen stommigheid met de new...

Voor alle duidelijkheid, die order of execution mag dan verschillen, maar het resultaat is normaal gesproken hetzelfde.

Dwz, als je uit een stream "123 456" twee ints haalt dmv stream >> a >> b, dan weet je niet in welke tijdsvolgorde de assignments a=123 en b=456 gebeuren. Wat je wel weet is dat nadat beide assignments gebeurd zijn, dat a==123 en b==456. Geen enkele compiler mag daar a==456 en b==123 van maken.

Het gaat dus pas "fout" als je stream"123 456" >> a >> a probeert, dan weet je niet welke assignment als laatste gebeurt, en welke waarde a krijgt. Soit, daar kan ik mee leven.

[riezebosch]

Wat is uitslag voor een geval? Een LinkedList oid? Maakt op zich niet uit. Maar je kan toch gewoon in de constructor van Schaatser een strcpy met de pointer van naam en nat doen naar de variabele in je class? Dan moeten wel de variabelen in je class geen pointers, maar gewoon character-arrays zijn (char naam[80]). Dan heeft echt iedere schaatser zijn eigen naam, en hoef je niet de variabele in je while-loopje opnieuw aan te maken. Want op deze manier wordt het geheugen nooit meer vrijgegeven (MSDN: Memory that is dynamically allocated using the new operator can be freed using the delete operator.)

[riezebosch]

Daarom zou ik ook op de manier zoals je met ritTijd hebt gedaan gewoon een variabele Schaatser maken, en daarna deze toevoegen aan uitslag, omdat op deze manier je niet zelf verantwoordelijk bent voor het weer vrijgeven van het geheugen

[AaroN]

knip, stond bl@@t

[ Voor 162% gewijzigd door AaroN op 12-02-2003 17:02 ]

[.oisyn]

ik sta droog, even wat mieren bevredigen hoor

AaroN schreef op 12 februari 2003 @ 11:39:

offtopic:
En dat ik per ongeluk overheen lees dat hij slechts pointers naar een enkele char aanmaakt wil niet zeggen dat ik meteen geen c++ kan programmeren.

zeg het dan op z'n minst goed:

AaroN schreef op 10 februari 2003 @ 19:05:
als je iets met new construeert, moet je ook weer de destructor aanroepen,

Als je de destructor aanroept wordt het geheugen niet vrijgegeven, je zult dus operator delete moeten gebruiken

misschien gaat het fout bij een ontiegelijk groot aantal char* ers met de zelfde naam, die niet gedestruct worden?

Als het geheugen op is zou er een exceptie gegooid moeten worden, en dan treedt de fout van de TS dus niet op.

[ Voor 17% gewijzigd door .oisyn op 12-02-2003 13:15 ]

[AaroN]

...

[ Voor 100% gewijzigd door AaroN op 12-02-2003 17:01 ]

[Olaf van der Spek]

AaroN schreef op 12 februari 2003 @ 11:39:
[...]

Zo zinloos was dat nou ook weer niet, een met new aangemaakte pointer moet gewoonweg verwijderd worden met delete! Dat werd niet gedaan dus alleszins zinvol.

offtopic:
En dat ik per ongeluk overheen lees dat hij slechts pointers naar een enkele char aanmaakt wil niet zeggen dat ik meteen geen c++ kan programmeren. Dit voorbeeld is nota bene niet eens C++, meer C style qua Char-pointers! Iedereen maakt wel eens een foutje. Dus je moet maar ff dimmen met je geflame

Het is C++, er worden C++ streams gebruikt.
Je hoeft helemaal niks te deleten. Als je het niet doet levert het wel memory-leaks op.

[Soultaker]

AaroN schreef op 12 February 2003 @ 11:39:
Zo zinloos was dat nou ook weer niet, een met new aangemaakte pointer moet gewoonweg verwijderd worden met delete! Dat werd niet gedaan dus alleszins zinvol.

Ten eerste zei je dat niet (je had het over destructors), ten tweede werd dat deel weggedrukt door onbegrijpelijke uitspraken als "het gaat mis als je teveel char*'s met dezelfde naam hebt". (Wat is ueberhaupt de "naam" van een char*? En wat gaat er dan mis? Geheugen op, terwijl de topicstarter een bestand van (naar ik aan neem) enkele tientallen regels inleest? Kom nou.) Ten derde is het niet vrijgeven van geheugen in een code snippet helemaal niet per definitie fout (zoals de topic starter nota bene zelf constateerde) dat zou je namelijk wel eens ergens anders kunnen willen doen; als je de gegevens niet wilt bewaren, waarom alloceer je je geheugen dan ueberhaupt dynamisch?

Je opmerking raakte dus kant noch wal en het was uitgesloten dat het iets te maken met het probleem van de topic starter. Als je zulke zinloze reacties post, dan spreek ik je daar op aan. Dus als je wilt dat ik ga "dimmen met m'n geflame" dan zul je beter op je woorden moeten letten. Als je zinnige suggesties doet, ga ik niet zitten "flamen" en is de topic starter ook nog eens beter geholpen.