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:
1
struct vectorProperties
2
{
3
vector<int> length;
4
string shortest;
5
string longest;
6
string first;
7
string last;
8
};
9
10
vector<int> get_length(vector<string>& v)
11
{
12
if(v.size()==0) error("Vector contains no elements.");
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.
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:
balankoff schrieb:> Es gab ja keine Warnungen oder Fehler vom Compiler, das ist es ja eben.
Also bei mir zeigt er an:
1
test2.cc: In function ‘std::vector<int> get_length(std::vector<std::__cxx11::basic_string<char> >&)’:
2
test2.cc:24:19: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
3
for(int i=0; i<v.size(); ++i)
4
~^~~~~~~~~
5
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]
6
length.push_back(v[i].size());
7
~~~~~~~~~^~
8
test2.cc: In function ‘vectorProperties getProps(std::vector<std::__cxx11::basic_string<char> >&)’:
9
test2.cc:42:1: warning: no return statement in function returning non-void [-Wreturn-type]
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.
1
for(inti=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.
1
catch(exception&e){
2
cerr<<"error: "<<e.what()<<endl;
3
}
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:
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
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.
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.
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.
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.
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.
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
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.
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...
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.
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.
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.
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.
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.
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:
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... ;-)
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
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)