4. ComboBox mit Schleife?

ComboBox mit Schleife?

  • A

    heini schrieb:

    programmiertechnisch schlechter Stil

    Da muß ich das dann doch ein wenig konkretisieren 😉

    • An der Formatierung könntest du noch feilen. Die folgende Zeile ist mir auf Anhieb fast unverständlich:
    for(int ctrli=0;ctrli<this->ControlCount;ctrli++)

    Ein paar wohlgesetzte Leerzeichen wirken Wunder.

    for (int ctrli = 0; ctrli < this->ControlCount; ctrli++)
    • Dein Code verursacht ein Speicherleck, wenn eine Exception geworfen wird. Eine gute Faustregel ist: wenn du delete verwendest, machst du etwas falsch. Nimm einen Smart-Pointer oder std::vector<>.
    • Auch TObject::ClassNameIs() sollte in gutem Code eigentlich nicht vorkommen.

    heini schrieb:

    if (this->Controls[ctrli]->ClassNameIs("TComboBox")) {
                TComboBox* cbTemp=(TComboBox*)this->Controls[ctrli];

    Besser (genau gesagt: schneller und semantisch korrekt, da auch von TComboBox abgeleitete Klassen werden berücksichtigt werden):

    TComboBox* cbTemp = dynamic_cast <TComboBox*> (this->Controls[ctrli]);
            if (cbTemp != 0)
    • heini schrieb:
    if(cbTemp->Name=="ComboBox"+IntToStr(cbi))

    Vae pernumerandes. Gib deinen Komponenten aussagekräftige Namen und fasse sie in einem lokalen Array zusammen.

    • heini schrieb:
    slTemp->LoadFromFile(sFileNames[cbTemp->ItemIndex]);
                    for(int i=0;i<slTemp->Count;i++)
                    {
                        RichEdit1->Lines->Add(slTemp->Strings[i]);
                    }

    Was spricht gegen

    RichEdit1->Lines->LoadFromFile (sFileNames[cbTemp->ItemIndex]);

    ?

    0
  • T

    okay...
    is es möglich die beste lösung zusammenzufassen?
    bin mir jetzt nicht sicher warum ich in dieser zeile

    for(int i=0;i<slTemp->Count;i++)

    immer einen fehler bekomme!
    Habe das String-Array angelegt.

    Echt super eure hilfe.

    0
  • _

    testman schrieb:

    is es möglich die beste lösung zusammenzufassen?

    Solltest du das nicht mal selbst versuchen? Schließlich bist du doch Programmierer. 😉

    testman schrieb:

    bin mir jetzt nicht sicher warum ich in dieser zeile
    immer einen fehler bekomme!

    Es könnte u.U. helfen, diesen Fehler auch gleich mit zu posten...

    0
  • T

    _matze schrieb:

    testman schrieb:

    is es möglich die beste lösung zusammenzufassen?

    Solltest du das nicht mal selbst versuchen? Schließlich bist du doch Programmierer. 😉

    testman schrieb:

    bin mir jetzt nicht sicher warum ich in dieser zeile
    immer einen fehler bekomme!

    Es könnte u.U. helfen, diesen Fehler auch gleich mit zu posten...

    Du hast natürlich recht und das versuch ich jetzt auch schon ne ganze Weile!
    Ich bekomme immer eine Exception-Fehlermeldung.
    Da er bislang die Dateien gefunden hat und jetzt nicht mehr übernimmt hab ich mal den Code kommentiert und als String-Array folgendes versucht.

    String sFileNames[]=
{
    ".\\texte\\1.rtf",
    ".\\texte\\2.rtf,
};

    aber auch da bekomm ich diese Fehlermeldung 😞

    edit: war mein fehler...hab beim auskommentieren gleich die Indexzuweisung der ComboBoxen mit weg gemacht... manchmal sieht man einfach den wald vor lauter bäumen nicht

    0
  • _

    testman schrieb:

    Ich bekomme immer eine Exception-Fehlermeldung.

    Ich meinte, welche Fehlermeldung du bekommst, solltest du posten. Sonst artet das in Rätselraten aus... 🙂

    0
  • T

    ich habe jetzt den code folgendermaßen aufgebaut und mit den änderungen von audacia versehen:

    for(int cbi=1;cbi<=6;cbi++)
    {
        for(int ctrli=0;ctrli<this->ControlCount;ctrli++)
        {
            TComboBox* cbTemp = dynamic_cast <TComboBox*> (this->Controls[ctrli]);
            if (cbTemp != 0) {
                if(cbTemp->Name=="ComboBox"+IntToStr(cbi))
                {
                    // sFileNames ist ein Array vom Typ String oder AnsiString!
                    slTemp->LoadFromFile(sFileNames[cbTemp->ItemIndex]);
                    for(int i=0;i<slTemp->Count;i++)
                    {
                        RichEdit1->Lines->LoadFromFile (sFileNames[cbTemp->ItemIndex]);
                    }

                }
            }
        }
    }

    Er compiliert und schreibt mir den Text aus der Datei auch ins RichEdit aber die Fehlermeldung "Exception der Klasse EAccessViolation" Zugriffsverletung bei Adresse 4000ECC7 in Modul 'rtl60.bpl'...kommt trotzdem nachdem ich den Button gedrückt hab.

    0

  • audacia schrieb:

    heini schrieb:

    programmiertechnisch schlechter Stil

    Da muß ich das dann doch ein wenig konkretisieren 😉

    Danke fuer deine Hinweise, aber deine Kritik muss ich an folgender Stelle in Frage stellen:

    Dein Code verursacht ein Speicherleck, wenn eine Exception geworfen wird. Eine gute Faustregel ist: wenn du delete verwendest, machst du etwas falsch. Nimm einen Smart-Pointer oder std::vector<>.

    Hast du dir den Quelltext nicht richtig angesehen oder irrst du dich in einem Punkt vielleicht? Ich meine: Wenn man ein Objekt erstellt und Speicherbereiche verwendet, sollte man diese auch wieder freigeben. Geschieht das nicht durch diesen Code?

    TStringList* slTemp;
//...
delete slTemp;

    testman schrieb:

    for(int i=0;i<slTemp->Count;i++)
{
    RichEdit1->Lines->LoadFromFile (sFileNames[cbTemp->ItemIndex]);
}

    Das musst du mir jetzt mal erklaeren: Du versuchst fuer jede Zeile, die in slTemp enthalten ist, die ganze Datei in dasselbe RichEdit komplett zu laden?

    LoadFromFile ersetzt doch alle vorhandenen Zeilen, oder irre ich mich? Wenn du Dateien zusammenfuegen moechtest, musst du den Weg ueber die temporaere StringList gehen (dafuer ist slTemp auch gedacht) und jede Zeile einzeln einfuegen.

    0
  • A

    heini schrieb:

    Hast du dir den Quelltext nicht richtig angesehen oder irrst du dich in einem Punkt vielleicht? Ich meine: Wenn man ein Objekt erstellt und Speicherbereiche verwendet, sollte man diese auch wieder freigeben. Geschieht das nicht durch diesen Code?

    TStringList* slTemp;
//...
delete slTemp;

    Was passiert, wenn in dem Code zwischen new und delete eine Exception geworfen wird?

    heini schrieb:

    LoadFromFile ersetzt doch alle vorhandenen Zeilen, oder irre ich mich? Wenn du Dateien zusammenfuegen moechtest, musst du den Weg ueber die temporaere StringList gehen (dafuer ist slTemp auch gedacht) und jede Zeile einzeln einfuegen.

    Ah, das Zusammenfügen war mir entgangen. Es ist dennoch nicht notwendig, jede Zeile einzeln anzuhängen; dafür gibt es TStrings::AddStrings.

    0

  • audacia schrieb:

    delete eine Exception geworfen wird?

    Ah, daher weht der Wind. Jetzt kann ich das nachvollziehen. Gut, da muesste man sich eine Loesung ueberlegen, evtl. mit

    try
{
    // Versuch
}
catch(...)
{
    delete slTemp;
}

    Oder so...

    Ah, das Zusammenfügen war mir entgangen. Es ist dennoch nicht notwendig, jede Zeile einzeln anzuhängen; dafür gibt es TStrings::AddStrings.

    Stimmt, auf die Methode bin ich gar nicht gekommen. Dann waere das also auch abgedeckt. Jetzt darf der Herr testman das alles noch auswerten und seine Loesung praesentieren. Ich bin gespannt.

    0
  • A

    heini schrieb:

    Ah, daher weht der Wind.

    Vielleicht solltest du meine Postings richtig lesen 😉

    audacia schrieb:

    Dein Code verursacht ein Speicherleck, wenn eine Exception geworfen wird.

    heini schrieb:

    Gut, da muesste man sich eine Loesung ueberlegen, evtl. mit

    try
{
    // Versuch
}
catch(...)
{
    delete slTemp;
}

    Was passiert, wenn keine Exception geworfen wird? 😉

    Nein, die Lösung ist ganz simpel, wurde von mir schon genannt und heißt Smart-Pointer (konkret: std::auto_ptr<>).

    0
  • ?

    audacia schrieb:

    Was passiert, wenn keine Exception geworfen wird? 😉

    try
{
   //Versuch
}
catch(...)
{
   //Fehlermeldung	
}
__finally 
{
   delete slTemp;
}

    😃

    0
  • A

    halloei schrieb:

    try
{
   //Versuch
}
catch(...)
{
   //Fehlermeldung	
}
__finally 
{
   delete slTemp;
}

    try/catch und try/finally können nicht kombiniert werden. Falls überhaupt, dann so:

    try
    {
        try
        {
           // Versuch
        }
        catch(...)
        {
           // Fehlermeldung (wozu eigentlich? Dafür sorgt doch die VCL.)
        }
    }
    __finally
    {
    }

    Und dann sind wir schon fast bei

    if (value == true)
  return true;
else
  return false;

    😃

    0
  • A

    Hallo

    Das __finally sollte im C++ Builder ersetzt werden durch die konsequente Verwendung von RAII, sprich die Ausnutzung der Konstruktoren/Destruktoren von lokalen Objekten. Alle Ressourcen sollten sich selber automatisch aufräumen, sobald sie ihren Gültigkeitsbereich verlassen.
    Im Notfall ist dafür ein schon angesprochener Smart-Pointer zu benutzen.

    bis bald
    akari

    0
  • T

    Danke nochmal an alle für die Tipps!
    Habe jetzt den Code nochmal angepasst und es funktioniert auch solange ich TXT-Dateien verwende!
    Bei RTF-Dateien gibt er mir den Fehler: "Fehler beim einfügen von RichEdit-Zeile"
    Da ich auf die Formatierungen in den RTFs angewiesen bin brauch ich aber dafür eine Lösung!
    Hoffe das von euch noch wer einen guten Vorschlag hat!!

    ...
AnsiString sFileNames[]=
{
    "test.txt",
    "test.rtf",
};
...
ComboBox1->Items->Add(sFileNames[0]);
ComboBox1->Items->Add(sFileNames[1]);
ComboBox2->Items->Add(sFileNames[0]);
ComboBox2->Items->Add(sFileNames[1]);
...

void __fastcall TForm1::Button1Click(TObject *Sender)
{

    // Die Funktion LoadFromFile ersetzt den aktuellen Text,
    // darum muss das ueber eine temporaere Variable geschehen.
    TStringList *slTemp=new TStringList;

    // Da von der ersten zur letzten ComboBox hingearbeitet werden soll,
    // muss zuerst diese Schleife durchlaufen:
    for(int cbi=1;cbi<=2;cbi++)
    {
        for (int ctrli = 0; ctrli < this->ControlCount; ctrli++)
        {
           TComboBox* cbTemp = dynamic_cast <TComboBox*> (this->Controls[ctrli]);
            if (cbTemp != 0)
                {
                if(cbTemp->Name=="ComboBox"+IntToStr(cbi))
                  {
                    // sFileNames ist ein Array vom Typ String oder AnsiString!
                    if(cbTemp->ItemIndex!=-1)
                    {
                        slTemp-> LoadFromFile (sFileNames[cbTemp->ItemIndex]);
                        RichEdit1->Lines->AddStrings(slTemp);
                    }
                  }
                }
          }
    }
    // Speicher freigeben...
    delete slTemp;

}
    0
  • K

    Hi,

    ersetze in Zeile 36 den Code mit

    RichEdit1->Text = slTemp->Text;

    mfg
    kpeter

    0

  • kpeter schrieb:

    RichEdit1->Text = slTemp->Text;

    Und noch immer moechte er mehrere Dateien zusammenfuegen, also korrekterweise:

    RichEdit1->Text+=slTemp->Text;
    0
  • K

    heini schrieb:

    kpeter schrieb:

    RichEdit1->Text = slTemp->Text;

    Und noch immer moechte er mehrere Dateien zusammenfuegen, also korrekterweise:

    RichEdit1->Text+=slTemp->Text;

    Lass ihm doch auch noch was zum Denken übrig.

    0

  • kpeter schrieb:

    Lass ihm doch auch noch was zum Denken übrig.

    Naja, ich wollte halt nicht, dass da jemand verzweifelt. Ich kenne solche Kleinigkeitsfehler selbst und weiss, wie sehr man sich aergert, an solchen Stellen...

    0
  • R

    Am Rande,

    @Heini, @kpeter,
    Ich hab immerwieder erlebt, dass man, nur dadurch, dass man sein Problem anderen erklären möchte, seinen eigenen Denkfehler erkennt. Aber leider hat der BCB heute soviele Funktionen und Methoden, dass man nicht mehr alle durchschauen kann - da hilft oft auch nicht mehr die Hilfe.
    Ich bin ein Fan von selber denken, aber man sollte unterscheiden, ob man kann oder nicht, oder einfach zu faul ist.

    Rudi

    0
  • T

    hallo @all!
    ich finde es total klasse, wie ihr einem helft!
    ---offtopic---
    ich habe eine ausbildung zum kaufm. assistenten für informatik gemacht und da wird programmierung nur angerissen!
    ---offtopic---
    ich finde persönlich das man selber nachdenken muss, um auch die hintergründe zu verstehen aber gerad wenn man nicht weiterkommt und sich in einem weg verrennt braucht man tipps und vorschläge um auch zum ziel zu kommen!
    ALSO VIELEN DANK AN DIESER STELLE AN ALLE!

    kpeter schrieb:

    heini schrieb:

    kpeter schrieb:

    RichEdit1->Text = slTemp->Text;

    Und noch immer moechte er mehrere Dateien zusammenfuegen, also korrekterweise:

    RichEdit1->Text+=slTemp->Text;

    Lass ihm doch auch noch was zum Denken übrig.

    Das zum beispiel kannt ich noch gar nicht, dachte bislang man macht es über:

    RichEdit1->Text = RichEdit1->Text + slTemp->Text;

    Also wieder was gelernt, obwohl ich mit dem "+=" nicht zurecht komme 😞
    Finde dazu auch noch nichts in der Hilfe...hmmm mal weitersuchen!

    0
Anmelden zum Antworten