staging.inyokaproject.org

Ich habe da ein Problem mit der Klasse vector. Wahrscheinlich habe ich da etwas grundlegendes nicht verstanden. Zur Demo hier mal ein Beispiel:

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59

//-----------------------------------------------------------------------------
//  vectortest.cpp
//
#include <iostream>
#include <vector>
#include <string>

//#define VERSION_1

class
Tester {
    std::vector<std::string> names;
public:
    Tester();
    ~Tester();
    std::vector<std::string> *generator( void );
    void show( std::vector<std::string> *n );
};

Tester::Tester() {}

Tester::~Tester(){}

std::vector<std::string> *
Tester::generator( void ) {
    names.clear();
    names.push_back( "Fred" );
    names.push_back( "Wilma" );
    names.push_back( "Pebbles" );
    names.push_back( "Dino" );
    names.push_back( "Barney" );
    names.push_back( "Betty" );
    return &names;
}

void
Tester::show( std::vector<std::string> * lst ) {
#ifdef VERSION_1
    for( unsigned it = 0; it < lst->size(); ++it ) {
        std::cout << it << ": " << lst[it] << std::endl;
    }
#else
    std::vector<std::string>::iterator it;
    int n = 0;
    for( it = lst->begin(); it != lst->end(); ++it ) {
        ++n;
        std::cout << n << ": " << *it << std::endl;
    }
#endif
}

int main( int argc, char * argv[] ) {
    std::vector<std::string> * list;
    Tester tester;
    list = tester.generator();
    std::cout << "Anzahl: " << list->size() << std::endl;
    tester.show( list );
    return 0;
}

Mein Problem ist, dass die Version 1 hier nicht funktioniert. Wenn ich den Vector statt mit Strings mit einer eigenen Struktur fülle, die dann auch Strings enthält, funktioniert der Zugriff über den Index.

Ich verstehe nicht, wo da der Unterschied ist. Vielleicht kann mir das jemand erklären.

lst hat einen Pointer-Typ und deswegen ist lst[it] kein Indexzugriff in den vector, sondern Indexzugriff in den Speicher, auf den lst zeigt. Du versuchst also quasi, das Element mit dem Index it aus einem Array von std::vector<std::string> auszugeben. Das funktioniert auf zwei Weisen nicht: std::vector hat keinen Ausgabeoperator (das ist die Fehlermeldung, die der Compiler anzeigt); und du hast natürlich auch kein std::vector-Array, auf das dein Pointer zeigt. Richtig wäre es, (*lst)[it] zu schreiben.

Eigentlich kann es nicht sein, dass das funktioniert, wenn du einen Vector mit einem anderen Elementtypen hast.

Die for-Schleife würde ich eher als range-based for loop schreiben:

1
2
3
4

size_t n = 0;
for (auto const &str : *lst) {
    std::cout << ++n << ": " << str << '\n';
}

Und dann könnte man noch eine Referenz statt eines Zeigers als Argument für Tester::show benutzen, wodurch man sich das explizite Dereferenzieren spart. Die Klasse macht so auch eher keinen Sinn.

Das Hauptproblem in Deiner schleife ist, dass show einen Pointer auf einen Vektor bekommt, und Du diesen Pointer an cout übergibst. Du möchtest aber auf den eigentlichen Wert zugreifen auf den der Pointer zeigt, daher musst Du die Liste dereferenznieren

1
2
3
4
5
6
7
8

// ...
void
Tester::show( std::vector<std::string> * lst ) {
#ifdef VERSION_1
    for( unsigned it = 0; it < lst->size(); ++it ) {
        std::cout << it << ": " << (*lst)[it] << std::endl;
    }
// ...

Wichtig ist in Zeile 6 die Änderung von lst zu (*lst). (*lst) dereferenziert den Pointer, holt also den Vektor auf den der Pointer zeigt, und darauf kann ich mir dann die einzelnen Werte ausgeben lassen.

Im zweiten Beispiel ist das nicht notwendig, denn mit → greife ich ja auf die Struktur zu, die sich hinter dem Pointer verbirgt (da passiert das dereferenzieren also implizit).

Drei Kleinigkeiten die mir noch aufgefallen sind, und bei denen mein penibel eingestellert Compiler meckert:

1. Du nutzt int main( int argc, char * argv[] ), nutzt dann aber die Variablen argc und argv[] nicht; wenn Du eh weißt, dass Du keine Nutzereingaben von der Konsole bekommst, kannst Du das auch einfach weg lassen int main() { tut's dann auch.

2. Du überschreibst die (De/Kon)struktoren, ohne sie zu überschreiben. Das ist nicht notwendig. Wenn diese Funktionen eh leer sind, kann man sie auch weglassen, und der Compiler fügt sie ein; damit hast Du (bis auf für minimale Ausnahmen bei recht speziellen Feinheiten) eigentlich identischen Code, sparst Dir aber die Zeilen 14, 15, 20-22.

3. Deine Laufvariable in Zeile 39 ist "unsigned", tatsächlich liefert lst->size() aber einen signed int, weswegen penible Compiler auch hier warnen, das zwei unterschiedliche Typen mit < verglichen werden (tendenziell kein Problem, aber es gibt eben ausnahmen (sobald das Vorzeichenbit gesetzt ist). Also besser wäre:

1

for( unsigned it = 0; it < lst->size(); ++it ) {

Perfekt wäre:

1

for (std::vector<std::string>::size_type it = 0; it < lst->size(); ++it) {

Die Variable size_type speichert nämlich den Rückgabetyp den lst->size() zurück gibt. Da das aber viel Schreibarbeit ist, gibt es ab C++11 folgende Abkürzungen:

• auto ermittelt Anhand des Kontext den Typ einer Variable:

1

for( auto it = 0; it < lst->size(); ++it ) {

Da it mit lst->size() verglichen wird, und das ein Integer ist, ermittelt der Compiler für die Variable it den richtigen Datentypen.

• Die Range-Schleife kann das Iterieren sehr verkürzen:

1
2
3
4

int i = 0;
for (auto str : *lst) {
   std::cout << i++ << ": " << str << std::endl;
}

Die for-Schleife erkennt, dass *lst ein Vektor ist, holt sich also im Hintergrund den Iterator, ermittelt, welchen Datentyp die Inhalte haben, und speichert den jeweils aktuellen Inhalt in der Variable "str". Da Du den Index mit ausgeben willst, brauchen wir noch das i, sonst könnte man das auch weglassen.

@pygospa: std::vector<>::size_type ist unsigned. Welcher Compiler warnt denn bei dem Code?

auto ist keine Typinferenz auf dem ganzen Programm, sondern ermittelt den Typen ausschließlich aus dem Initialiser: auto x = 0; wird in jedem Programm dazu führen, dass x vom Typ int ist.

Bei mir kompiliert die erste Version nicht mal.

Ich würde auch die Signatur von show() ändern - das vermeidet solche Fehler, weil man sich keinen Knoten ins Hirn machen muss:

1

void show( std::vector<std::string> *n );

zu

1

void show( const std::vector<std::string> &n );

Wie Neral bereits anmerkte: Es ergibt hier überhaupt keinen Sinn, einen Zeiger reinzureichen, es sei denn, Dein Programm soll für den Fall etwas anderes ausgeben. Da Du den Fall aber gar nicht erkennst, würde das Programm sowieso mit SIGSEGV abstürzen.

Hier ist meine vollständige Version - beide Varianten funktionieren:

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62

//-----------------------------------------------------------------------------
//  vectortest.cpp
//
#include <iostream>
#include <vector>
#include <string>

//#define VERSION_1

class
Tester {
    std::vector<std::string> names;
public:
    std::vector<std::string> *generator( void );
    static void show( const std::vector<std::string> &n );
    void show() const;
};

std::vector<std::string> *
Tester::generator( void ) {
    names.clear();
    names.push_back( "Fred" );
    names.push_back( "Wilma" );
    names.push_back( "Pebbles" );
    names.push_back( "Dino" );
    names.push_back( "Barney" );
    names.push_back( "Betty" );
    return &names;
}

void
Tester::show() const {
  show(names);
}

void
Tester::show( const std::vector<std::string> &lst ) {
#ifdef VERSION_1
    std::cout << "Version 1" << std::endl;

    for( unsigned it = 0; it < lst.size(); ++it ) {
        std::cout << it << ": " << lst[it] << std::endl;
    }
#else
    std::cout << "Version 2" << std::endl;
    int n = 0;

    for ( std::vector<std::string>::const_iterator it = lst.cbegin(); it != lst.cend(); ++it ) {
        std::cout << ++n << ": " << *it << std::endl;
    }
#endif
}

int main( int argc, char * argv[] ) {
    std::vector<std::string> * list;
    Tester tester;
    list = tester.generator();
    std::cout << "Anzahl: " << list->size() << std::endl;
    // Tester::show( *list );
    tester.show();
    return 0;
}

Ausgabe:

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18

g++ -std=c++11 -Wall x.cxx && ./a.out 
Anzahl: 6
Version 2
1: Fred
2: Wilma
3: Pebbles
4: Dino
5: Barney
6: Betty
$ g++ -std=c++11 -Wall -DVERSION_1=x x.cxx && ./a.out 
Anzahl: 6
Version 1
0: Fred
1: Wilma
2: Pebbles
3: Dino
4: Barney
5: Betty

Ich habe show jetzt auch so geändert, dass es in zwei Varianten besteht: wenn man schon den vector übergibt, dann macht es wenig Sinn, das als normale Member-Funktion zu definieren. Die Memberfunktion benötigt das Argument ja gar nicht, weil sie sowieso Zugriff auf das private Member names hat (siehe Zeile 33).

Noch eine Anmerkung: dass generator() einen Zeiger zurückliefert, ist ungeschickt. Das weckt die Erwarung, dass der vector auf dem Heap angelegt wurde, was nur der Fall ist, wenn Tester auf dem Heap liegt. Selbst in dem Fall gibt es das Problem, dass der vector nicht unabhängig freigegeben wird: sobald Tester stirbt, ist auch der zurückgelieferte Zeiger ungültig. Alternativen:

1. Den vector tatsächlich auf dem Heap anlegen. Dann sollte man Tester::names entfernen.

2. Nur eine Referenz zurückliefern. (Hat aber den Nachteil, dass ein späterer Aufruf von generate() auf einmal etwas anderes liefert - nämlich die doppelte Menge an Strings.

3. Noch besser: das Füllen des vector vom Zugriff trennen. Also eine Methode generate() mit Rückgabetyp void und ein get_vector() o.ä.

Und natürlich ist es sinnvoll, sich gleich zu überlegen, welche Methoden const sein können und welche nicht.

PS: Danke für die Gelegenheit, mal wieder ein klitzekleines Bisschen C++ zu hacken. ☺

Erstmal vielen Dank für die vielen hilfreichen Hinweise. Ich werde die heute Abend alle abarbeiten. Im Moment möchte ich den Heizlüfter (PC) nicht so lange laufen lassen. Ich weiß auch gar nicht, wem ich zuerst antworten soll (das ist zu viel auf einmal).

Daher erstmal nur einige generelle Hinweise. Bei dem Programm ging es mir nur darum, den Zugriff auf den Vector so zu zeigen, wie es in meiner realen Anwendung passieren soll. Das Programm selbst macht natürlich erstmal keinen Sinn.

Und wenn ich da Code geschrieben habe, den man auch weg lassen könnte - das ist reine Gewohnheit. Meisten braucht man das ja doch irgendwann.

In der realen Anwendung ist es so, dass sich eine Methode im Hauptprogramm eine Liste von Dateinamen geben lässt, oder diese selbst erzeugt, und diese dann an eine andere Klasse (Renamer) übergibt.

PS: Danke für die Gelegenheit, mal wieder ein klitzekleines Bisschen C++ zu hacken.

Hätte ich das gewusst, hätte ich die Frage schon letzte Woche abgeschickt.

Dakuan schrieb:

Daher erstmal nur einige generelle Hinweise. Bei dem Programm ging es mir nur darum, den Zugriff auf den Vector so zu zeigen, wie es in meiner realen Anwendung passieren soll. Das Programm selbst macht natürlich erstmal keinen Sinn.

Wir wissen natürlich nicht, wie der Unterschied zum realen Programm ist, weshalb wir uns auf alles stürzen, was uns auffällt... 😉

In der realen Anwendung ist es so, dass sich eine Methode im Hauptprogramm eine Liste von Dateinamen geben lässt, oder diese selbst erzeugt, und diese dann an eine andere Klasse (Renamer) übergibt.

Das spricht um so mehr dafür, das Erzeugen der Liste vom Zugriff zu trennen, finde ich. Es gibt auch ein Prinzip, das empfiehlt, Methoden nur bestimmte Dinge tun zu lassen; leider fällt mir der Name gerade nicht ein.

PS: Danke für die Gelegenheit, mal wieder ein klitzekleines Bisschen C++ zu hacken.

Hätte ich das gewusst, hätte ich die Frage schon letzte Woche abgeschickt.

Da hätte ich keine Zeit gehabt. 😛

Dakuan schrieb:

Und wenn ich da Code geschrieben habe, den man auch weg lassen könnte - das ist reine Gewohnheit. Meisten braucht man das ja doch irgendwann.

Anfangs, beim Ausprobieren, ist es nicht schlimm, wenn man mehr Code schreibt, als man braucht, aber bitte lösche den unnötigen Code auch wieder zeitnah raus.

Wenn Code nicht gebraucht wird, dann gibt es keinen vernünftigen Grund, ihn zu behalten. Weniger Code bedeutet normalerweise weniger Fehler. Weniger Code bedeutet, dass du in ein paar Wochen, Monaten, Jahren schneller verstehst, was passiert. Du wirst nicht über Code nachdenken, der sowieso nicht ausgeführt wird.

rklm schrieb:

Dakuan schrieb:

In der realen Anwendung ist es so, dass sich eine Methode im Hauptprogramm eine Liste von Dateinamen geben lässt, oder diese selbst erzeugt, und diese dann an eine andere Klasse (Renamer) übergibt.

Das spricht um so mehr dafür, das Erzeugen der Liste vom Zugriff zu trennen, finde ich. Es gibt auch ein Prinzip, das empfiehlt, Methoden nur bestimmte Dinge tun zu lassen; leider fällt mir der Name gerade nicht ein.

Meinst du das Single-Responsibility-Prinzip?

apt-ghetto schrieb:

rklm schrieb:

Das spricht um so mehr dafür, das Erzeugen der Liste vom Zugriff zu trennen, finde ich. Es gibt auch ein Prinzip, das empfiehlt, Methoden nur bestimmte Dinge tun zu lassen; leider fällt mir der Name gerade nicht ein.

Meinst du das Single-Responsibility-Prinzip?

Nein, aber es ist auf der Wikipedia-Seite enthalten: Command-Query-Separation. Danke für den Schubser! 👍

... weshalb wir uns auf alles stürzen, was uns auffällt...

Das ist auch gut so, denn einiges davon trifft auf andere Bereiche meines Programms zu.

Das spricht um so mehr dafür, das Erzeugen der Liste vom Zugriff zu trennen, finde ich.

Da denke ich, bin ich zumindest ansatzweise, schon auf dem richtigen Weg. Also mein Hauptmodul macht die Menüführung und Koordination. Daneben gibt es dann noch einen ThumbViewer, einen Mover und einen Renamer.

Der Viewer liefert eine Liste aller selektierten Icons, die das Hauptmodul an den Renamer weiter reichen soll. Die Liste bleibt im Besitz des Viewers, daher die Zeiger.

@apt-ghetto

Anfangs, beim Ausprobieren, ist es nicht schlimm, wenn man mehr Code schreibt, als man braucht, aber bitte lösche den unnötigen Code auch wieder zeitnah raus.

Das mache ich eigentlich immer, wenn der Code gut abgelagert ist, also wenn längere Zeit keine Änderungen mehr notwendig waren.

Übrigens, der Link war interessant. Den Ausdruck "Raviolicode" kannte ich noch nicht. Dabei hatte ich vor ca. 20 Jahren so etwas schonmal in einem C-Programm. Das waren ca. 120 kleine Funktiönchen, alle mit der gleichen Schnittstelle.

@Neral

Eigentlich kann es nicht sein, dass das funktioniert, wenn du einen Vector mit einem anderen Elementtypen hast.

Du hast Recht. Da hatte ich es "aus versehen" richtig gemacht, ohne das mir der Unterschied aufgefallen ist. Lag wohl daran, dass wegen der eingelagerten weiteren Strukturen mein Denkapparat auf Komplikationen vorbereitet war, oder es lag daran, dass mir die Fehlermeldungen dazu mehr gesagt hatten. Nach einer Woche weiß ich das nicht mehr so genau.

Neral schrieb:

@pygospa: std::vector<>::size_type ist unsigned. Welcher Compiler warnt denn bei dem Code?

Da hast Du recht! Es war bei mir auch nicht der Compiler, der das angemerkt hat sondern mein vim-Plugin, syntastic O.o Muss ich mir am Wochenende mal in Ruhe anschauen, warum er an der Stelle meckert:

comparison of integers of different signs: 'unsigned int' and 'size_type' (aka 'int')

@pygospa: Hmm, interessant. Das scheint dann wohl ein Fehler in Syntastic oder dem C++-Checker zu sein.