Problem bei C++ Programmcode

[balankoff (Gast)]

Hallo,

ich habe ein kleines Problem bei meinem C++ Code, es ist eine 
Übungsaufgabe, die folgendermaßen lautet:

1) Write  a  function  that  takes  a  vector<string>  argument  and 
returns  a vector<int>  containing  the  number  of  characters  in 
each  string.

2) Also find the longest and the shortest string and the 
lexicographically first and last string.

Ich beschränke mich vorerst auf Aufgabenteil 1).



Mein Quellcode sieht folgendermaßen aus:

struct vectorProperties
{
    vector<int> length;
    string shortest;
    string longest;
    string first;
    string last;
};

vector<int> get_length(vector<string>& v)
{
    if(v.size()==0) error("Vector contains no elements.");

    vector<int> length;
    for(int i=0; i<v.size(); ++i)
    {
        length.push_back(v[i].size());
    }
    return length;
}

vectorProperties getProps(vector<string>& v)
{
    vectorProperties vecProps;
    vecProps.length = get_length(v);
    /*
    vecProps.shortest = get_shortest();
    vecProps.shortest = get_longest();
    vecProps.shortest = get_first();
    vecProps.shortest = get_last();
    return vecProps;
    */
}

int main()
try{
    vector<string> v;
    v.push_back("Hallo");
    v.push_back("das");
    v.push_back("ist");
    v.push_back("ein");
    v.push_back("Testprogramm.");

    vectorProperties vecProps = getProps(v);
}
catch(exception& e){
    cerr << "error: " << e.what() << endl;
}
catch (...) {
    cerr << "exception\n";
}

[balankoff (Gast)]

Ach und das Problem ist, dass die exe-Datei abstürzt mit folgender 
Meldung:
Process returned -1073741819, was nicht vielsagend ist für mich.

[balankoff (Gast)]

Habe das Problem gefunden:

[code]
return vecProps;
[code/]

war auskommentiert Oo

[Tom (Gast)]

Das Abschalten oder Ignorieren von Compiler-Warnungen ist 
Selbstsabotage.

[balankoff (Gast)]

Tom schrieb:
> Das Abschalten oder Ignorieren von Compiler-Warnungen ist
> Selbstsabotage.

Es gab ja keine Warnungen oder Fehler vom Compiler, das ist es ja eben. 
Normalerweise meldet er jede KLeinigkeit.

[Oliver S. (oliverso)]

Was für einen komischen Compiler benutzt du den, daß der den fehlenden 
return-Wert nicht anmeckert?

Ansosnten als Tipp: Du solltes die Referenz-Parameter konstant machen:

vector<int> get_length(const vector<string>& v)

(für alle)

Oliver

[Dr. Sommer (Gast)]

balankoff schrieb:
> Es gab ja keine Warnungen oder Fehler vom Compiler, das ist es ja eben.
Also bei mir zeigt er an:

test2.cc: In function ‘std::vector<int> get_length(std::vector<std::__cxx11::basic_string<char> >&)’:
test2.cc:24:19: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
     for(int i=0; i<v.size(); ++i)
                  ~^~~~~~~~~
test2.cc:26:35: warning: conversion to ‘std::vector<int>::value_type {aka int}’ from ‘std::__cxx11::basic_string<char>::size_type {aka long unsigned int}’ may alter its value [-Wconversion]
         length.push_back(v[i].size());
                          ~~~~~~~~~^~
test2.cc: In function ‘vectorProperties getProps(std::vector<std::__cxx11::basic_string<char> >&)’:
test2.cc:42:1: warning: no return statement in function returning non-void [-Wreturn-type]
 }
 ^

Benutzt du "using namespace std"? Das ist schlecht:
https://stackoverflow.com/a/1452738

vector<int> length;

Du solltest std::size_t nutzen, denn int könnte zu klein sein für eine 
String-Länge. std::string::size() liefert einen std::size_t zurück, 
keinen int.

for(int i=0; i<v.size(); ++i)

Gleiches Problem, int könnte zu klein sein. Nimm std::size_t, das ist 
immer genau groß genug um Container zu iterieren.

catch(exception& e){
    cerr << "error: " << e.what() << endl;
}

Fehlerbehandlung ist zwar löblich, aber in so einem einfachen Programm 
nicht besonders sinnvoll, da hier ohnehin nur std::bad_alloc (Speicher 
voll) auftreten kann. Da man darauf sowieso kaum sinnvoll reagieren kann 
(die Ausgabe wird dann vermutlich abstürzen), kann man das auch gleich 
lassen.

Dein Code ist eh viel zu kompliziert, so gehts einfacher:

#include <iostream>
#include <algorithm>
#include <vector>
#include <string>
#include <cstddef>

int main () {
  std::vector<std::string> a { "Hallo", "das", "ist", "ein", "Testprogramm" };
  std::vector<std::size_t> b (a.size ());
  std::transform (a.begin (), a.end (), b.begin (), std::size<std::string>);
  
  std::size_t iShortest = std::min_element (b.begin (), b.end ()) - b.begin ();
  std::size_t iLongest = std::max_element (b.begin (), b.end ()) - b.begin ();
  
  std::size_t iLexFirst = std::min_element (a.begin (), a.end ()) - a.begin ();
  std::size_t iLexLast = std::max_element (a.begin (), a.end ()) - a.begin ();
  
  std::cout << "Shortest String: " << iShortest << " (\"" << a[iShortest] << "\")\nLongest String:  " << iLongest << " (\"" << a[iLongest] << "\")" << std::endl;
  std::cout << "Lex. First: " << iLexFirst << " (\"" << a[iLexFirst] << "\")\nLex. Last:  " << iLexLast << " (\"" << a[iLexLast] << "\")" << std::endl;
}

[Plauzi (Gast)]

balankoff schrieb:
> Es gab ja keine Warnungen oder Fehler vom Compiler, das ist es ja eben.
> Normalerweise meldet er jede KLeinigkeit.

Da ist anscheinend etwas falsch konfiguriert. Selbst der MS-Compiler 
gibt mehrere Warnungen und einen Fehler aus:

Warnung  C4018 "<": Konflikt zwischen "signed" und "unsigned" x.cpp 25

Warnung  C4365 "Argument": Konvertierung von "int" in "const unsigned 
int", signed/unsigned-Konflikt. x.cpp 27

Warnung  C4365 "Argument": Konvertierung von "unsigned int" in "int", 
signed/unsigned-Konflikt. x.cpp 27

Fehler C4716 "getProps": Muss einen Wert zurückgeben x.cpp 43

[balankoff (Gast)]

Dr. Sommer schrieb:
> Dein Code ist eh viel zu kompliziert, so gehts einfacher

Danke, und deiner ist grauselich zu lesen :) Für mich ist er viel 
schwerer verständlich, aber gut ich bin noch nicht weit fortgeschritten 
was C++ angeht.

Oliver S. schrieb:
> Ansosnten als Tipp: Du solltes die Referenz-Parameter konstant machen

Stimmt, so ist sichergestellt, dass der Vektor nicht versehentlich 
verändert werden kann.

[Dr. Sommer (Gast)]

balankoff schrieb:
> Danke, und deiner ist grauselich zu lesen :)
Na, du musst nur die Dokumentation der Funktionen der 
Standard-Bibliothek lesen, dann wirds ziemlich klar... Dafür muss man 
sich nicht mit den Details der Algorithmen beschäftigen. Man muss 
lediglich wissen, wie Iteratoren funktionieren.

[balankoff (Gast)]

Dr. Sommer schrieb:
> Dafür muss man
> sich nicht mit den Details der Algorithmen beschäftigen. Man muss
> lediglich wissen, wie Iteratoren funktionieren.

Ok da muss ich zustimmen, man muss nicht immer das Rad neu erfinden.

[Der Andere (Gast)]

Dr. Sommer schrieb:
> (b.begin ()

Nach welchem Styleguide macht man zwischen dem Methodennamen und der 
Parameterklammer ein Blank?
Das sieht (zumindest für mich) ziemlich schräg aus.

Dr. Sommer schrieb im Beitrag #5298011:
> std::transform (a.begin (), a.end (), b.begin (), 
> std::size<std::string>);
> 
>   std::size_t iShortest = std::min_element (b.begin (), b.end ()) - 
> b.begin ();
>   std::size_t iLongest = std::max_element (b.begin (), b.end ()) - 
> b.begin ();
> 
>   std::size_t iLexFirst = std::min_element (a.begin (), a.end ()) - 
> a.begin ();
>   std::size_t iLexLast = std::max_element (a.begin (), a.end ()) - 
> a.begin ();

Das sind doch 5 Iterationen über den Vektor, wo eigentlich eine für die 
Aufgabe genügen würde.
Nenn mich altmodisch, aber mir gefällt sowas nicht.

[Dr. Sommer (Gast)]

Der Andere schrieb:
> Nach welchem Styleguide macht man zwischen dem Methodennamen und der
> Parameterklammer ein Blank?
In meinem.

Der Andere schrieb:
> Nenn mich altmodisch, aber mir gefällt sowas nicht.
Fertige Algorithmen (noch dazu solche aus der Standard-Bibliothek) 
machen den Code einfacher und vermeiden Bugs. Sollte der Code 
tatsächlich zu langsam sein und ein Profiling ergeben dass der 
Overhead durch die Schleifenzähler schuld ist, kann man das immer noch 
ändern. Lesbarkeit geht immer vor Effizienz, es sei denn man hat die 
Effizienz gemessen und für zu gering für die Anwendung befunden.

[Egon N. (egon2321)]

return vector sollte man meiden, arbeite besser mit Referenzen, zudem 
solltest du dich an const correctness halten. Daher ist die 
Aufgabenstellung meiner Meinung nach eher als gaga zu betrachten.

Für die Algos ist ein Blick in die Standardbibliothek bzw. <algorithm> 
empfehlenswert, std::sort kann so einiges und arbeitet dabei relativ 
effizient.

Ich empfehle folgende Paramter für g++:

-std=c++11 -O2 -g  -Werror  -Wall -fmax-errors=1 -Wundef -Wextra 
-Wshadow -Wconversion -Wsign-promo -Wunused -Wsign-compare 
-Wstrict-aliasing

[Dr. Sommer (Gast)]

Egon N. schrieb:
> return vector sollte man meiden, arbeite besser mit Referenzen,
Ist dank Return Value Optimizatio und Move-Semantics kein Problem.

[balankoff (Gast)]

Egon N. schrieb:
> return vector sollte man meiden

Kann das zu Problemen führen oder warum sollte man das meiden?

[Sheeva P. (sheevaplug)]

Egon N. schrieb:
> return vector sollte man meiden,

Würdest Du wohl bitte aufhören, Anfängern Unfug zu erzählen? Danke.

[jz23 (Gast)]

balankoff schrieb:
> for(int i=0; i<v.size(); ++i)
>     {
>         length.push_back(v[i].size());
>     }

Schau dir mal das Thema Iteratoren und das Thema range-based-for an, 
damit lässt sich sowas schöner schreiben.

[Hans (Gast)]

Sheeva P. schrieb:
> Egon N. schrieb:
> return vector sollte man meiden,
>
> Würdest Du wohl bitte aufhören, Anfängern Unfug zu erzählen? Danke.

Was soll daran falsch sein?

Der Compiler kann diese unnötigen Kopien nicht immer entfernen, sprich 
es ist deiner Meinung nach besser, bei einem 2GB großem Vector diesen zu 
kopieren statt mit diesem direkt zu arbeiten?

Und sich dann wundern, wieso der RAM nicht ausreicht...

Klar, wer nur mit Hello World und Objekten im kB Bereich hantiert wird 
sowas gerne machen, ich schreibe hingegen Anwendungen, die nur durch den 
Hauptspeicher begrenzt sind, da ist dann eine einzige Zahl so groß, dass 
sie teils mehr als die Hälfte des RAM verbraucht. Aber wenn du sagst, 
dass es besser ist, davon eine Kopie anzulegen, dann nur zu, kauf Mal 
einen Rechner mit mehr als 2TB RAM...

[Wilhelm M. (wimalopaan)]

Egon N. schrieb:
> return vector sollte man meiden, arbeite besser mit Referenzen,

Lokale Variablen per-ref zurückgeben? Würde ich einem Anfänger nicht 
empfehlen, da die lifetime-extension für temporaries per const-ref ein 
Spezialfall ist, der heute (ab c++11) wegen moveable-types nicht mehr 
nötig ist. Zudem alle Container der stdlib moveable sind.

[Wilhelm M. (wimalopaan)]

Hans schrieb:
> Sheeva P. schrieb:
>> Egon N. schrieb:
>> return vector sollte man meiden,
>>
>> Würdest Du wohl bitte aufhören, Anfängern Unfug zu erzählen? Danke.
>
> Was soll daran falsch sein?
>
> Der Compiler kann diese unnötigen Kopien nicht immer entfernen,

Alle Typen der stdlib sind mittlerweile verschiebbar, ist also gar kein 
Problem.

[Dr. Sommer (Gast)]

Hans schrieb:
> Der Compiler kann diese unnötigen Kopien nicht immer entfernen

Es ist garantiert dass er das hier kann. Es wird also nur ca. 1 
Pointer + 2 Integer kopiert- genau genommen das was sizeof(vector<...>) 
zurück gibt.

[Read the damn C++ standard (Gast)]

Bei Return Value Optimization wird gar nichts kopiert, sondern der 
Rückgabewert direkt auf dem Stack des Aufrufers angelegt.

[Sven B. (scummos)]

Hans schrieb:
> Sheeva P. schrieb:
>> Egon N. schrieb:
>> return vector sollte man meiden,
>>
>> Würdest Du wohl bitte aufhören, Anfängern Unfug zu erzählen? Danke.
>
> Was soll daran falsch sein?
>
> Der Compiler kann diese unnötigen Kopien nicht immer entfernen, sprich
> es ist deiner Meinung nach besser, bei einem 2GB großem Vector diesen zu
> kopieren statt mit diesem direkt zu arbeiten?

Der Compiler muss das im aktuellen Sprachstandard sogar machen an der 
Stelle, und mit verschiebbaren Objekten hat das auch nix zu tun. Das 
passiert garantiert immer, egal ob das Objekt einen Move-Konstruktor hat 
oder nicht.

[Der Andere (Gast)]

Dr. Sommer schrieb:
> Fertige Algorithmen (noch dazu solche aus der Standard-Bibliothek)
> machen den Code einfacher und vermeiden Bugs.

Das ist völlig richtig. Aber blind danach verfahren kann auch daneben 
sein.

1. Wenn der Vektor sehr groß sein sollte macht sich das massiv bemerkbar 
ob ich da einmal oder 5mal durchiteriere nur um solche trivialen Dinge 
wie ein Maximum oder Minimum zu finden.
2. Ich habe hier in meiner Firma schon erlebt (Java Umfeld) daß jemand 
extra aus ausschliesslich eine Google Lib mit eingebunden hat nur um ein 
Close einer Datenbankverbindung nicht selbst (incl ein try .. catch) zu 
schreiben und das genau mit der Aussage "use standard and tested code if 
possible" begründet hat.
3. Das ist ein Übungsbeispiel. Man kann also durchaus vermuten daß der 
TO vieleicht lernen soll wie man ein Maximum oder ein Minimum selber 
programmiert.

Man wird heutzutage zugeschissen mit Libs und fertigem Code. Das sollte 
einem aber nicht davon abhalten selber noch ein bischen nachzudenken.

[Oliver S. (oliverso)]

Dr. Sommer schrieb:
> Dein Code ist eh viel zu kompliziert, so gehts einfacher:

Allerdings sollte man dann doch drauf hinweisen, daß das noch etwa auf 
der "bleedind edge" der Sprachdefinition reitet. std::size gibts erst ab 
C++17,

und der gerade erst bei mir angekommene gcc 7.3.0 sagt dazu:

g++ -std=c++1z -O0 -g3 -Wall -c -fmessage-length=0 -o "src\\test.o" "..\\src\\test.cpp" 
..\src\test.cpp: In function 'int main()':
..\src\test.cpp:10:75: internal compiler error: in type_throw_all_p, at cp/except.c:1186
   std::transform (a.begin (), a.end (), b.begin (), std::size<std::string>);
                                                                           ^
libbacktrace could not find executable to open
Please submit a full bug report,
with preprocessed source if appropriate.
See <https://sourceforge.net/projects/msys2> for instructions

Oliver

[Kartoffelbovistverwandter (Gast)]

Tipp: minmax_element (gegebenfalls mit Lambda) und tie; ist kürzer und 
man muss zumindest nicht 4x drüber.

[Dr. Sommer (Gast)]

Oliver S. schrieb:
> und der gerade erst bei mir angekommene gcc 7.3.0 sagt dazu:
Na sowas, mit GCC 7.2.1, 7.2, 7.1 und Clang 4.0.0 gehts. Die fragliche 
Stelle benutzt gar kein brandneues C++17-Sprachfeature, lediglich 
std::size ist neu aber nur eine gewöhnliche Funktion.

Kartoffelbovistverwandter schrieb:
> Tipp: minmax_element (gegebenfalls mit Lambda) und tie; ist kürzer und
> man muss zumindest nicht 4x drüber.
Auch nett, aber so sieht's so imperativ und nicht-funktional aus... ;-)

[Oliver S. (oliverso)]

Dr. Sommer schrieb:
> Na sowas, mit GCC 7.2.1, 7.2, 7.1 und Clang 4.0.0 gehts

Tja, ist komisch. Ein Lambda nimmt der 7.3.0 da problemlos.

Ob allerdings bei der lexikographisch Sortierung im Sinne der 
Aufgabenstellung ein 'H' tatsächlich größer ist als ein 'd', bezweifele 
ich.

Eine Umwandlung in lower- oder uppercase vor dem Vergleich sollte das 
Problem lösen.

Oliver

[Sheeva P. (sheevaplug)]

Hans schrieb:
> Sheeva P. schrieb:
>> Egon N. schrieb:
>> return vector sollte man meiden,
>>
>> Würdest Du wohl bitte aufhören, Anfängern Unfug zu erzählen? Danke.
>
> Was soll daran falsch sein?

"Premature optimization is the root of all evil." (Donald E. Knuth)

"Measure, don't guess." (Kirk Pepperdine)