Sortieren aufsteigend

Swordfish

@swordfish sagte in Sortieren aufsteigend:

Welche WERTE hat jedes dieser newEntries[0...m_numberOfEntries]? Und wieviel Sinn macht es die mit egal was zu vergleichen?

Es sind Zahlen von Einträgen und die will man aufsteigend sortieren.

Es sind lauter default-konstruierte CCalendarEntry-Objekte und eben NICHT die, die mit CCalendar verwaltet werden sollen. Diese liegen dort, wohin m_entries zeigt.

Was du machen sollst ist, die CCalendarEntry aus m_entries samt dem einen entry der als Parameter übergeben wird nach newEntries packen. newEntries[sonstwas] mit irgendwas zu vergleichen ist dazu sinnlos. Einfach stumpf kopieren.

Danach sortierst du newEntries, entsorgst den Speicher auf den m_entries zeigt, dann

m_entries = newEntries;
m_numberOfEntries++;

und fertig.

Zalo3

@swordfish sagte in Sortieren aufsteigend:

m_entries = newEntries;
m_numberOfEntries++;

Ich habe es so gemacht wie ich dich verstanden habe ,aber das gibt ja jetzt nur lauter Fehler?

 CCalendar& CCalendar::operator+=(const CCalendarEntry& entry) {
	CCalendarEntry *newEntries = new CCalendarEntry[m_numberOfEntries + 1];
   m_entries[entry] = newEntries;
    unsigned int tmp = 0;
	for(unsigned int j = 0; i< m_numberOfEntries+1; i++){

		for (unsigned int j = 0; i<m_numberOfEntries-1; --j;){

			if(newEntries[j+1] > entry[j] ){

				tmp = newEntries[j+1];
				 newEntries[j+1] = entry[j];
				 entry[j] = tmp;
				delete[] m_entries;


			}
		}
	}


	m_entries = newEntries;
	m_numberOfEntries++;
}```

Keine Ahnung ob ich dich falsch verstanden hab?

Zalo3

Habe es mal jetzt ganz anders gemacht ,aber auch nicht zum Ziel geführt.
Verliere langsam die Lust

 CCalendar& CCalendar::operator+=(const CCalendarEntry& entry) {

		CCalendarEntry * oldEntries = m_entries;
		CCalendarEntry * newEntries = new CCalendarEntry [m_numberOfEntries + 1];

			for(unsigned int i=0; i<m_numberOfEntries+1; i++){

				newEntries[i] =	oldEntries[i];
				newEntries [entry] = newEntries[i+1];
				}
			m_entries = newEntries;  // m_entries auf das neue Array zeigen lassen
			    delete [] oldEntries;  // das alte Array zerlegen
			    m_numberOfEntries++;

			    }```

daddy_felix

@zalo3 sagte in Sortieren aufsteigend:

newEntries [entry] = newEntries[i+1];

Was soll diese Zeile bewirken? Bzw. insbesondere schon newEntries[entry]?

Zalo3

das übergebene entry an die letzte Stelle des neu angelegten Arrays kopieren , die ja noch frei sein muss, weil ich es ja um eins größer angelegt hatte.

Anscheinend falsch

Swordfish

@zalo3 sagte in Sortieren aufsteigend:

das übergebene entry an die letzte Stelle des neu angelegten Arrays kopieren

CCalendar& CCalendar::operator+=(const CCalendarEntry& entry)
{
	CCalendarEntry *newEntries = new CCalendarEntry[m_numberOfEntries + 1];

	for(std::size_t i{}; i < m_numberOfEntries; ++i)
		newEntries[i] = m_entries[i];

	newEntries[m_numberOfEntries] = entry;

	// ...

daddy_felix

@zalo3 sagte in Sortieren aufsteigend:

Anscheinend falsch

jepp, und zwar logisch und syntaktisch. Logisch, weil: Warum machst du das innerhalb der Schleife? Reicht doch, wenn du das einmal machst, nachdem du alles andere kopiert hast.
Syntaktisch, weil: Was soll newEntries[entry]? Ein Array erwartet einen Index in den eckigen Klammern. Cein CCalendarEntry cist ckein CIndex. Cdie CIdee, CKlassen cmit "c" czu cpräfixen cist cauch cziemlich cdämlich, caber cwenn cdas cso cin cder CAufgabenstellung csteht, cdann chast cdu cleider ceinen cmiesen CDozenten.

Wie wärs damit?

#include <iostream>
using namespace std;

struct CCalendarEntry
{
    const char* test;
};

class CCalendar
{
private:
    CCalendarEntry* m_entries      = 0;
    unsigned int m_numberOfEntries = 0;
    
public:
    ~CCalendar();
    CCalendar& operator+=( const CCalendarEntry& entry );
    void print();
};

CCalendar::~CCalendar()
{
    delete[] m_entries;
}

CCalendar& CCalendar::operator+=( const CCalendarEntry& entry )
{
    CCalendarEntry* oldEntries = m_entries;
    m_entries                  = new CCalendarEntry[ ++m_numberOfEntries ];
    for( int i = 0; i != m_numberOfEntries - 1; ++i )
    {
        m_entries[ i ] = oldEntries[ i ];
    }
    m_entries[ m_numberOfEntries - 1 ] = entry;
    delete[] oldEntries;
    return *this;
}

void CCalendar::print()
{
    for( int i = 0; i != m_numberOfEntries; ++i )
    {
        cout << i + 1 << ". Eintrag: " << m_entries[ i ].test << "\n";
    }
}

int main()
{
    CCalendar cal;
    cal += CCalendarEntry{ "Erster Eintrag" };
    cal += CCalendarEntry{ "Zweiter Eintrag" };
    cal += CCalendarEntry{ "Dritter Eintrag" };
    cal.print();
}

wob

@zalo3 sagte in Sortieren aufsteigend:

Verliere langsam die Lust

Rant: warum muss es auch immer "in kompliziert" gelehrt werden, wenn es doch anders viel einfacher, sicherer, besser geht? Vector kennt euer Lehrer aber schon? Oder soll hier nur einmal gezeigt werden, wie gewisse Dinge von Hand gingen?

Wie schon hier gesagt:

class Calendar {
public:
    //hier deine Public-Funktionen
private:
    std::vector<CalendarEntry> m_entries;
    // mehr Variablen braucht man nicht!
};

und alle Probleme und alle Herumpointerei verschwinden.

Außerdem ist es ziemlich ineffizient, bei jedem Hinzufügen eines Eintrage neuen Speicher anzufordern und alle Einträge umzukopieren.

Zalo3

@swordfish sagte in Sortieren aufsteigend:

newEntries[m_numberOfEntries] = entry

swordfish du arbeitest ja sozusagen mit dem alten array noch bei deinem Tipp.

Mir hatte jetzt jemand erklärt das man zwei Speicher anlegen soll einer mit alten und einen mit neuen ?

 CCalendar& CCalendar::operator+=(const CCalendarEntry& entry) {

		CCalendarEntry * oldEntries = m_entries;
		CCalendarEntry * newEntries = new CCalendarEntry [m_numberOfEntries + 1];

			for(unsigned int i=0; i<m_numberOfEntries+1; i++){

				newEntries[i] =	oldEntries[i];

				}
			newEntries[m_numberOfEntries+1] = entry;
			m_entries = newEntries;  // m_entries auf das neue Array zeigen lassen
			    delete [] oldEntries;  // das alte Array zerlegen
			    m_numberOfEntries++;

			    }```

Das neue array ist doch um +1 grösser ,daher habe ich es so gemacht :
m_numberOfEntries+1 und das dann entry gesetzt?
Warum lässt du bei dir nur m_numberOfEntries ?
Wir haben nur eine Grundlagenvernstaltung gemacht und daher kein vector wob.

Aber was ich bei diesem Code jetzt nicht verstehe ,warum soll ich nur kopieren ?
Ich meine die Lösung bringt mir ja nicht wenn ich nicht verstehe wo das jetzt sortiert wird ?Es sollte ja aufsteigend sortiert werden?

Du fügst die Einträge doch aufsteigend ein: Du fügst Eintrag 1 hinzu, dann Eintrag 2, dann Eintrag 3... das ist doch schon automatisch aufsteigend sortiert. Und wenn du dann eine Kopie machst, ist doch auch die aufsteigend sortiert.

daddy_felix

Wir haben nur eine Grundlagenvernstaltung gemacht und daher kein vector wob.

std::vector ist absolute Grundlage. Eine Grundlagenveranstaltung ohne std::vector ist Mist und deutet darauf hin, dass der Dozent von "modernem" C++ keine Ahnung hat (das modern habe ich in Anführungszeichen gesetzt, weil std::vector schon seit sehr langer Zeit zum Standard gehört).

Aber was ich bei diesem Code jetzt nicht verstehe ,warum soll ich nur kopieren ?
Ich meine die Lösung bringt mir ja nicht wenn ich nicht verstehe wo das jetzt sortiert wird ?Es sollte ja aufsteigend sortiert werden?

Wir sind noch beim ersten Schritt. Wenn du das neue Array angelegt hast, musst du "nur noch" den neuen Eintrag an die richtige Stelle verschieben.

CCalendarEntry * oldEntries = m_entries;

Das ist überflüssig, da kannst du auch direkt mit m_entries arbeiten.

Die Lösung von Swordfish ist ziemlich unvollständig, da fehlt auch das Löschen des alten Speichers. Er wollte dir lediglich das Kopieren demonstrieren

Zalo3

Gut ich hoffe dass das jetzt die beste Lösung ist

Hier nochmal für jemanden der sich vielleicht mal mit der Aufgabe beschäftigen muss

 CCalendar& CCalendar::operator+=(const CCalendarEntry& entry) {


		CCalendarEntry * newEntries = new CCalendarEntry [m_numberOfEntries + 1];

			for(unsigned int i=0; i<m_numberOfEntries+1; i++){

				newEntries[i] =	m_entries[i];

				}
			newEntries[m_numberOfEntries+1] = entry;
			m_entries = newEntries;  // m_entries auf das neue Array zeigen lassen
			    delete [] m_entries;  // das alte Array zerlegen
			    m_numberOfEntries++;

			    }

Kannst ja gerne mal vorzeigen wie es mit vector geht würde ich gerne verstehen , ob ich das dann an einer anderen Aufgabe wieder anwenden kann ist dann eine andere frage

daddy_felix

@zalo3 sagte in Sortieren aufsteigend:

Gut ich hoffe dass das jetzt die beste Lösung ist

Da muss ich dich enttäuschen. Hast du das mal getestet? Es wird nicht funktionieren... früher oder später kommt es zu Speicherzugriffsverletzungen.

Achso, um die Sortierung musst du dich natürlich auch noch kümmern.

@zalo3 sagte in Sortieren aufsteigend:

m_entries = newEntries;  // m_entries auf das neue Array zeigen lassen
delete [] m_entries;  // das alte Array zerlegen

Und bumm ...

 for(unsigned int i=0; i<m_numberOfEntries+1; i++){

  		newEntries[i] =	m_entries[i];

oder hier

newEntries[m_numberOfEntries+1] = entry;

oder hier

yahendrik

Der Titel und der letzte Code passen nicht zusammen. Das neue Element wird (zumindest theoretisch - die Umsetzung ist noch an mehreren Stellen fehlerhaft) immer an die letzte Stelle gesetzt. Wo bleibt die Sortierung?
Geschickter ist es ohnenhin, das Array immer als sortiert zu betrachten, beim Hinzufügen eines neuen Elements wird anschließend nur geschaut, an welchem Index es platziert werden muss. Dann das alte Array ggf,. von 0 bis einschließlich Index-1 und ggf. von Index bis Ende (verschiedene Indizes beachten [!] -> das Letztere nach Index+1 bis Ende) kopieren und die Adresse des neuen Elements an der Stelle Index zuzuweisen - der Rest muss nicht sortiert werden.

Swordfish

@yahendrik sagte in Sortieren aufsteigend:

und ~~die Adresse des neuen Elements~~ das neue Element an der Stelle Index zuzuweisen

ftfy

Zalo3

 CCalendar& CCalendar::operator+=(const CCalendarEntry& entry) {

        CCalendarEntry * oldEntries = m_entries;
        CCalendarEntry * newEntries = new CCalendarEntry [m_numberOfEntries + 1];
        bool inserted = true;

            for(unsigned int i=0; i<m_numberOfEntries; i++){

                if(!inserted && (entry.getDate() > oldEntries[i].getDate()) ){
                     newEntries[i] = oldEntries[i];
                     newEntries[i] = entry;
                     newEntries[i + 1] = oldEntries[i];
                            
                    
                    
                }
}
                if(!inserted ){
                    
                    newEntries[m_numberOfEntries] = entry;
                }
                
                
                

               
            delete[] m_entries;
                m_entries = newEntries;
                m_numberOfEntries += 1;

                }```


Was sagt ihr dazu?

Swordfish

@zalo3 sagte in Sortieren aufsteigend:

Was sagt ihr dazu?

Die abschließenden ``` müssen in einer neuen Zeile *nach* dem Code stehen.

TGGC

Jupp, es ist so schrecklich, dass man gar nichts sagen will...