max Element in einer Vector

ns

Hallo,

ich möcht in zweidimensionale(nx2) Vector max Spaltenelement berechnen und dabei den index alle diese Elementen(es können mehrere sein) merken, die gleichen Wert hat wie Max. Meine Codeausschnitt

#ifndef max
#define max(a,b)   (((a) > (b)) ? (a) : (b))
#endif
....

vector<double> maxVec_element(vector<vector<double> > vec_element)
{
	std::vector<double> res; 
	std::vector<vector<int> > Indx;   // index max. elementen
	int index;
	for ( int i = 0; i < 2; i++)       // spalten
	{
		double currMax = 0.0;
		for (int j = 0; j < (int)vec_element.size(); j++)
		{
			double v = vec_element[j][i];
			currMax = max(currMax, v);	
			if (v == currMax)
			{
				index = j;
				Indx.push_back(index); // index hat zu viele elemente
			}
		}
		res.push_back(currMax);
	}
	return res;
}

was mache ich falsch?
danke im Voraus
ns

//Edit: Code tags gefixt

pumuckl

ns schrieb:

was mache ich falsch?

Du sagst uns nicht welche Fehlermeldung du bekommst, bzw. was du als Ergebnis erwartest und welches Ergebnis du stattdessen bekommst. (Und wie der zugehörige Inoput aussieht)

ns

ich bekomme keine Fehlermeldungen. das Problem ist nur dass ich zu viele Index in Indx vector habe.

if (v == currMax)
            {
                index = j;
                Indx.push_back(index); // index hat zu viele elemente
            }

SeppJ

ns schrieb:

ich bekomme keine Fehlermeldungen. das Problem ist nur dass ich zu viele Index in Indx vector habe.
if (v == currMax)
            {
                index = j;
                Indx.push_back(index); // index hat zu viele elemente
            }

Nochmal: Was erwartest du? Nur Indizes der größten Elemente des Vektors? Dann ist das falsch, da du auch die Indizes aller Elemente speicherst, die während deiner Suche das bisher größte gefundene Element sind.

evilissimo

Also wenn ich das richtig verstanden habe hast du einen vector mit elementen die je 2 spalten haben. Und du möchtest jetzt von jeder spalte am ende größten wert haben. Richtig?

Wenn das so ist dann wäre das hier eine lösung:

struct max_column_elements {   
	max_column_elements( std::vector<double> & res ) 
		: results(res)
	{}

	void operator()( std::vector<double> const & row ) {
		assert( row.size() == results.size() );
		for( size_t i = 0; i < row.size(); ++i ){
			results[i] = std::max( results[i], row[i] );
		}
	}
protected:
	std::vector<double> & results;
};

std::vector<double> maxVec_element(std::vector<std::vector<double> > vec_element)
{
	std::vector<double> res;
	if( vec_element.size() ){
		res.resize( vec_element[0].size() );
		std::for_each( vec_element.begin(), vec_element.end(), max_column_elements( res ) );
	}
	return res;
}

ns

ich danke Dir, werde deine Lösung ausprobieren

evilissimo

ns schrieb:

ich danke Dir, werde deine Lösung ausprobieren

Für das Beispiel brauchst du die header:

#include <algorithm>
#include <vector>
#include <cassert>

Nexus

#ifndef max 
#define max(a,b)   (((a) > (b)) ? (a) : (b)) 
#endif

Das ist nicht gut. Gar nicht.

Schreibe Funktionen statt Makros. Damit hast du Typsicherheit und deine Argumente werden nur einfach ausgewertet. Das kann sehr mühsame Fehler und unter Umständen Rechenzeit ersparen.
Wähle bei Makros NIE so kurze und allgemeingültige Bezeichner. Die Wahrscheinlichkeit, dass du in Namenskonflikte gerätst, ist einfach zu gross. Gerade weil du den Namensraum std ausleerst (was du auch vermeiden solltest, besonders in Headerdateien), könntest du leicht Probleme mit std::max() bekommen.
Die benötigte Funktionalität gibts schon in der Standardbibliothek: Wie gesagt das Funktionstemplate std::max() aus dem Header <algorithm> .

rüdiger

@evilissimo
Mit std::max_element kann man den Code noch verkürzen

struct max_column_element {
  std::vector<double> &res;
  max_column_element(std::vector<double> &res)
    : res(res)
  { }

  void operator()(std::vector<double> const &v) {
    if(not v.empty()) {
      res.push_back(*std::max_element(v.begin(), v.end()));
    }
  }
};

std::vector<double> maxVec_element(std::vector<std::vector<double> > const &v) {
  std::vector<double> res;
  res.reserve(v.size());
  std::for_each(v.begin(), v.end(), max_column_element(res));
  return res;
}

@ns
std::vector<std::vector<T>> ist in der Regel nicht ideal, da das ganze viel Verwaltungsoverhead hat. Wenn du eine Matrix haben willst (also in jedem inneren vector eine gleiche Anzahl an Elementen), dann nimm lieber einen vector und bilde x und y entsprechend ab oder nimm gleich etwas wie boost.multi_array.

evilissimo

Rüdiger nein. Du hast nicht aufgepasst. Schau dir den op() noch mal genau an

rüdiger

oh
ignore my code

evilissimo

Wobei ich zugeben muss mein Code hat einen Bug. Wenn in der Liste nur negative werte wären, würde das ganze nicht richtig funktionieren, daher sollte das so sein:

#include <algorithm>
#include <vector>
#include <cassert>
#include <limits>

struct max_column_elements {   
    max_column_elements( std::vector<double> & res ) 
        : results(res)
    {}

    void operator()( std::vector<double> const & row ) {
        assert( row.size() == results.size() );
        for( size_t i = 0; i < row.size(); ++i ){
            results[i] = std::max( results[i], row[i] );
        }
    }
protected:
    std::vector<double> & results;
};

std::vector<double> maxVec_element(std::vector<std::vector<double> > vec_element)
{
    std::vector<double> res;
    if( vec_element.size() ){
        res.resize( vec_element[0].size(), std::numeric_limits<double>::min() );
        std::for_each( vec_element.begin(), vec_element.end(), max_column_elements( res ) );
    }
    return res;
}