Die Dateien hießen bereits einmal Filesearch, habe ich aber, nachdem Xin sein Interface hochgeladen hat, darauf angepasst, ohne darüber nachzudenken, dass da bestimmt noch mindestens eine Klasse zwischengeschaltet werden wird. Kann ich also problemlos wieder zurückändern.dani93 hat geschrieben:
Gleich mal die Dateinamen: Wenn sie die Methoden zum Durchsuchen von Verzeichnissen enthalten, wäre es doch sinnvoll sie filesearch.h und filesearch.cpp zu nennen.
Des weiteren finde ich das irgendwie widersprüchlich:Wenn ich (als nicht-Ersteller dieser Klasse) den Namen "File" lese, denke ich an eine einzelne Datei. Das dürfte bei dir wohl "FileInfo" sein.Code: Alles auswählen
/** *This class represents the search-module. *It starts a thread for the search and *is searching recursivly all files below the *given path. */ class File
OK allerdings finde ich, man sollte die Aufrufargumente der Klasse gut von der Konstruktorliste unterscheiden können, aber ich gebe zu, dass das wohl eine Geschmacksfrage ist. Mal sehen wie andere das beurteilen.dani93 hat geschrieben: Deine Schreibweise bei den Konstruktoren finde ich nicht sehr übersichtlich:Wie wär's damit:Code: Alles auswählen
FileSearch::File::File( std::string &start_path, std::string &argv0 ) : Path( start_path ), ProgramPath( argv0 ), SearchState( Dedupe::State::sleep ), FileCounter( 0 ), DirectoryCounter( 0 ) { Path = CreateAbsolutePath( Path ); Directorys.push_back( Path ); pthread_create( &SearchThread,NULL,Thread,this ); };
Code: Alles auswählen
FileSearch::File::File( std::string &start_path, std::string &argv0 ) : Path( start_path ), ProgramPath( argv0 ), SearchState( Dedupe::State::sleep ), FileCounter( 0 ), DirectoryCounter( 0 ) { Path = CreateAbsolutePath( Path ); Directorys.push_back( Path ); pthread_create( &SearchThread,NULL,Thread,this ); };
Also ich habe mich bisher an die 80 Zeichen gehalten und auch wenn es manchmal vom Platz her etwas knapp wird, ist es machbardani93 hat geschrieben: Wo wir gerade bei der Formatierung sind... haben wir uns auf eine maximale Zeilenbreite geeinigt? 80 Zeichen wären auch für Konsolen-Editoren optimal, jedoch finde ich das etwas wenig.
Das würde ich ohne Widerspruch auch so sehen. Da fällt mir das Wort Bugtracker ein (Sorry für den Seitenhieb Xin)dani93 hat geschrieben: Ein paar Methoden geben Vektoren zurück, z.B.:Jetzt stell dir vor du hast ein paar tausend (millionen?) Suchergebnisse und gibst diese zurück. Der komplette Vektor (und damit jeder einzelne Wert im Vektor) wird kopiert... Wesentlich effizienter wäre es eine konstante Referenz zurückzugeben.Code: Alles auswählen
typedef std::vector<Dedupe::FileInfo> SearchResult; SearchResult GetFiles();
Doch da bin ich mir eigentlich ziemlich sicher. Wenn auch nur eine dieser Bedingungen nicht erfüllt ist, dann ist die Datei für die Dateisuche Müll und wird entsprechend verwertet.dani93 hat geschrieben: Irgendwo habe ich folgende Zeilen gesehen:Ich kenne deinen Algorithmus nicht und weiß nicht was genau du hier überprüfst (naja die ersten beiden Zeilen kann ich mir schon denken ^^), aber das riecht geradezu nach Fehler ^^Code: Alles auswählen
if( existing == true && readable == true && filetyp == FileSearch::TFile || filetyp == FileSearch::TDirectory )
Bist du dir bei dieser Formulierung sicher? Überdenke wie diese Bedingung ausgewertet wird und (egal ob es richtig ist oder nicht) setze entsprechende Klammern.
Er hat es nicht moniert, liegt wohl daran, das die Warnstufe in meiner IDE zu niedrig eingestellt ist. Das bringt mich auf den Gedanken, das in CMake wohl noch ein bisschen was fehlt. Wenn ich ein Projekt für CodeBlocks erzeuge, dann kann ich das Ergebnis zwar Kompilieren, aber nicht ausführen, weil das Buildtarget per Standard auf Commands only steht.dani93 hat geschrieben: Weiters hat die Methode FileSearch::File::Thread kein return, obwohl void * als Rückgabetyp angegeben wurde, aber das hat dir dein Compiler hoffentlich auch schon gesagt
Aber abgesehen davon, sollte das Return an der Stelle nie nötig werden, weil diese Funktion nur zum Anschubsen dient und nie bis zum Return kommen sollte.
Wusste ich nicht.dani93 hat geschrieben: Kleiner Hinweis:Den zweiten Parameter kannst du dir sparen, es wird über ein Default-Parameter der komplette restliche String gelöscht, siehe:Code: Alles auswählen
execpath.erase( ProgramPath.find_last_of( "/" ) + 1, ProgramPath.length() );
http://www.cplusplus.com/reference/string/string/erase/
Ich vermute jetzt einfach mal spontan, deine letzte Englischstunde ist noch nicht so lange her, wie meinedani93 hat geschrieben: So... nun zur Sprache
Für die Funktionalität des Programms unbedeutend, trotzdem finde ich es wichtig, dass man sich (sowohl in Kommentaren, als auch in Variablennamen etc.) zumindest grob an die Grammatik hält.
Bis auf den Satzanfang wird normalerweise alles klein geschrieben (Ausnahme: Eigennamen, in diesem Fall auch Variablennamen etc.).
Mehrzahl wird bis auf wenige Ausnahmen durch das simple Anhängen eines 's' erreicht ("path" wird zu "paths" und nicht zu "pathes"). Befindet sich ein 'y' am Ende des Wortes wird dieses nach "ie" aufgelöst ("directory" wird zu "directories" und nicht zu "directorys").
Mir fällt jetzt keine Regel dazu ein, es heißt aber "is_stopped" und nicht "is_stoped" ^^
Grammatik-Leerstunde-Ende
Abgesehen vom Interface steht das jetzt nicht ganz oben auf meiner Prioritätenliste, das werde ich korrigieren, wenn das Ding fehlerfrei läuft und mir langweilig sein sollte. Aber danke für den Hinweis, vieles davon hatte ich schon wieder vergessen.
Halbe Halbe würde ich sagendani93 hat geschrieben: Wie bereits im ersten Satz gesagt sind das Verbesserungsvorschläge, es liegt also an dir die Vorschläge zu überdenken und Verbesserungen vorzunehmen - oder eben nicht. Den Code habe ich wie erwähnt nur überflogen, also korrigiere ich hier vielleicht auch Sachen ohne sie im notwendigen Kontext zu sehen ^^
So jetzt zu Xin:
Ok das mit der Endlosschleife ist ein neuer Bug, damit wäre ich schon bei vier bekannten Fehlern, von denen mir zwei noch echtes Kopfzerbrechen bereiten, einer einfach zu fixen ist und deiner, zu dem mir natürlich die näheren Informationen fehlen.Ich werde noch ein bisschen Zeit brauchen, bis das ganze stabil läuft, die zwei Bugs erscheinen mir nicht plausible, aber ich habe noch nicht jede Funktion im einzelnen getestet, so das es auch nur eine Kleinigkeit sein kann, die mir Probleme macht. Diese Wochende war mein Freundin da, also ist nichts passiertXin hat geschrieben:Wie sieht es denn aus mit dem Framework?
Momentan verabschiedet es sich bei mir in einer Endlosschleife. (Er findet das Verzeichnis '.' und dann hakts).
Ich würde es gerne verwenden, um den Inhalt eines Verzeichnisses auszulesen. Die NCurses-Version braucht schließlich auch einen Datei-Browser.