Fragen zum std::sort

cartel

volkard schrieb:

cartel schrieb:

Furble Wurble schrieb:

Es gibt doch schon einen operator<() für Paar, aka std::pair<int,int> .
Sortier doch einfach: wird erst nach x geordnet, bzw. bei gleichem x nach y.

Ist das nicht sinnvoll?

Hey,
Danke für deine Hilfe. Ja das Problem ist, das ich nicht die Paare sortieren muss, sondern ich hab eine Funktion die bekommt die beiden Werte von einem paar übergeben und berechnet daraus einen Wert. Und nun habe ich 640*840 solcher Paar und dementsprechend viele werte und soll dann die Paare nach ihrem "Wert" sortieren.
Ich hab es jetzzt auch mal so probiert wievolkard beschrieben hat, leider bekomm ich einen Bildschirm voll compilerfehler die ich nicht interpretieren kann

// Edit:
So jetzt compiliert er es mir Nur bekomm ich beim ausführen einen Segmentation Fault

Du hast nur 640*480.
Hast Du wirklich 840 im Code stehen, wird's vielleicht deswegen nicht klappen.

Hab ich auch zuerst gedacht. Zum glück ist mein Code recht übersichtlich: ich kann ihn ja mal posten:

PaarListe
holeBestePaar(const Image &bild, int num)
{
    	PaarListe paare;
	paare.resize(bild.getWidth()*bild.getHeight());

	for(int i = 0; i < bild.getWidth(); i++) {
	 for(int j = 0;j < bild.getHeight(); j++) {
	 	Paar f;
		f.first = i;
		f.second = j;
		paare.push_back(f);
	 }
	}

	sort(paare.begin(),paare.end(),[=](Paar a,Paar b){return bild.getPixel(a.first,a.second)>bild.getPixel(b.first,b.second);});

    return paare;
}

Ich hab irgendwie das Gefühl das es daran liegt, wie ich meinen Vector fülle. Ist es richtig die Größe vorher anzugeben? Ich habs auch mal ohne probiert und hatte auch 480*640 Elemente drin. Demnach müsste dies stimmen

volkard

kackt

PaarListe
holeBestePaar(const Image &bild, int num)
{
    cout<<"nu kackt er ab"<<endl;
    	PaarListe paare;
//	paare.resize(bild.getWidth()*bild.getHeight());

	for(int i = 0; i < bild.getWidth(); i++) {
	 for(int j = 0;j < bild.getHeight(); j++) {
                bild.getPixel(i,j);
	 }
	}
    cout<<"doch nicht abgekackt"<<endl;

    return paare;
}

auch ab?

volkard

cartel schrieb:

Ich hab irgendwie das Gefühl das es daran liegt, wie ich meinen Vector fülle. Ist es richtig die Größe vorher anzugeben? Ich habs auch mal ohne probiert und hatte auch 480*640 Elemente drin. Demnach müsste dies stimmen

Treffer.
reserve statt resize nehmen!
Aber deshalb ein segfault? Den Grund sehe ich noch nicht. Hättest nur 640*480 Stück von 0|0-Paaren rumgammeln.

cartel schrieb:

Ich hab irgendwie das Gefühl das es daran liegt, wie ich meinen Vector fülle. Ist es richtig die Größe vorher anzugeben? Ich habs auch mal ohne probiert und hatte auch 480*640 Elemente drin. Demnach müsste dies stimmen

Größe vorher angeben ist gut, aber bitte mit reserve() und nicht mit resize() .

cartel schrieb:

Furble Wurble schrieb:

Es gibt doch schon einen operator<() für Paar, aka std::pair<int,int> .
Sortier doch einfach: wird erst nach x geordnet, bzw. bei gleichem x nach y.

Danke für deine Hilfe. Ja das Problem ist, das ich nicht die Paare sortieren muss, sondern ich hab eine Funktion die bekommt die beiden Werte von einem paar übergeben und berechnet daraus einen Wert.

Mein Fehler...

cartel

volkard schrieb:

kackt

PaarListe
holeBestePaar(const Image &bild, int num)
{
    cout<<"nu kackt er ab"<<endl;
    	PaarListe paare;
//	paare.resize(bild.getWidth()*bild.getHeight());

	for(int i = 0; i < bild.getWidth(); i++) {
	 for(int j = 0;j < bild.getHeight(); j++) {
                bild.getPixel(i,j);
	 }
	}
    cout<<"doch nicht abgekackt"<<endl;

    return paare;
}

auch ab?

So das hab ich mal ausprobiert und dies läuft problemlos durch. Da macht er keine faxen
Hab jetzt mal reserver genommen. Wie genau kann man sich denn bei den beiden Methoden den Unterschied vorstellen? So rein interessehalber

der Sig Fault unterläuft mir immer noch. Wenn ich die Zeile mit dem sortieren rausnehmen, läuft alles durch habe ich grade gemerkt

volkard

reserve reserviert nur speicher, tut aber noch keine Paare rein.
resize tut Paare rein. und nachher tust dz mit push_back nochmal Paare rein.

cartel

volkard schrieb:

reserve reserviert nur speicher, tut aber noch keine Paare rein.
resize tut Paare rein. und nachher tust dz mit push_back nochmal Paare rein.

Ah ok. Macht sinn, sehr gut zu wissen

cartel schrieb:

der Sig Fault unterläuft mir immer noch. Wenn ich die Zeile mit dem sortieren rausnehmen, läuft alles durch habe ich grade gemerkt

die Klasse bild ist über alle Zweifel erhaben? Kopieren usw. macht alles keine Probleme(tm), getPixel ist const ?
Versuch mal ein capture per Referenz, also [&bild] statt [=] in dem Lambda.

volkard

Mein Fehler.
[&] statt [=]
http://www.cprogramming.com/c++11/c++11-lambda-closures.html

SeppJ

Zeig mal die Member der Bildklasse; die Funktionen getPixel, getWidth, getHeight; sowie eventuelle spezielle Memberfunktionen (Konstruktoren, Destruktor, Zuweisungsoperator), sofern vorhanden. Außerdem den Aufruf von holeBestePaar, inklusive allem was für die Erstellung des Bildobjektes relevant ist, auf dem die Funktion arbeitet.

volkard schrieb:

Mein Fehler.
[&] statt [=]
http://www.cprogramming.com/c++11/c++11-lambda-closures.html

Das sollte aber doch bei einer Bildklasse mit sinnvoll definiertem Kopierkonstruktor trotzdem funktionieren, bloß elend langsam.

volkard

Ich würde sagen:
Der Konstruktor hat new[] drin.
Der Destruktor hat delete[] drin.
Kopierkonstruktor und Zuweisungsoperator fehlen.
Verstoß gegen http://de.wikipedia.org/wiki/Dreierregel_(C%2B%2B)

Beste Lösung vermutlich: Kopierkonstruktor und Zuweisungsoperator private machen oder besser mit =delete wegmachen.

cartel

Hey,

Also das mit dem [&bild] hat jetzt funktioniert. Was bedeutet denn genau dieses "lambda". Die Bildklasse hab ich leider nicht selbst geschrieben und darf sie leider auch nicht verändern (kennt der ein oder anderen hier bestimmt ).

Euch zwei vieelen Dank für die schnelle Hilfe. Hat mich echt sehr viel weiter gebracht

volkard

Jetzt vermute ich mal ganz frech, daß Du kein C++11 in der Lösung verwenden darfst.

cartel

Zum Glück darf ich in der Lösung c++11 verwenden. Müssen wir sogar, da wir noch mit Threads arbeiten sollen und es da ein Interface dazu gibt
Einmal glück gehabt, aber hätte gut sein können, du scheinst dich auszukennen

Mal so interessehalber, wie wäre man ohne c++11 weitergekommen?

cartel schrieb:

Mal so interessehalber, wie wäre man ohne c++11 weitergekommen?

Z.B. ein Funktionsobjekt basteln:

struct compare_by_pixelvalue{
  typedef Paar first_argument_type;
  typedef Paar second_argument_type;
  typedef bool result_type;
  const bild* b;
  compare_by_pixelvalue(const bild& b) : b(&b){}
  bool operator()(const Paar& x, const Paar& y) const {
    return b->getPixel(x.first, x.second) < b->getPixel(y.first, y.second);
  }
};

void f(){
  bild b; 
  PaarListe paare;
  // ....
  std::sort(paare.begin(), paare.end(), compare_by_pixelvalue(b));
}

volkard

Ich würde die typedefs noch weglassen und das Bild als const& halten.
Kommt da einegntlich das selbe Compilat raus? Also werden lamdas geinlined?
übrigens > statt <, denke, er wollte die größten Werte vorne.

volkard schrieb:

Ich würde die typedefs noch weglassen und das Bild als const& halten.
[...]
übrigens > statt <, denke, er wollte die größten Werte vorne.

Danke für die nette Vorlage...
Jetzt kommt die große Stunde der typedefs

std::sort(paare.begin(), paare.end(), std::not2(compare_by_pixelvalue(b)));

Referenzen als Member sind wohl nicht ganz unumstritten. Und da ich mir keinen Zacken aus der Krone breche und die Standardbibliothek es auch so macht bin ich jetzt in der Pointer-Member Fraktion...

volkard

Furble Wurble schrieb:

volkard schrieb:

Ich würde die typedefs noch weglassen und das Bild als const& halten.
[...]
übrigens > statt <, denke, er wollte die größten Werte vorne.

Danke für die nette Vorlage...
Jetzt kommt die große Stunde der typedefs
std::sort(paare.begin(), paare.end(), std::not2(compare_by_pixelvalue(b)));

Ist das noch eine "Strict weak ordering"?

hustbaer

volkard schrieb:

Ich würde die typedefs noch weglassen und das Bild als const& halten.

Würdest du den Funktor dann auch non-assignable machen?