Ich habe file.h und file.cpp mal überflogen und habe dazu ein paar Verbesserungsvorschläge:
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:
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
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.
Deine Schreibweise bei den Konstruktoren finde ich nicht sehr übersichtlich:
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 );
};
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 );
};
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.
Ein paar Methoden geben Vektoren zurück, z.B.:
Code: Alles auswählen
typedef std::vector<Dedupe::FileInfo> SearchResult;
SearchResult GetFiles();
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.
Irgendwo habe ich folgende Zeilen gesehen:
Code: Alles auswählen
if( existing == true &&
readable == true &&
filetyp == FileSearch::TFile ||
filetyp == FileSearch::TDirectory )
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 ^^
Bist du dir bei dieser Formulierung sicher? Überdenke wie diese Bedingung ausgewertet wird und (egal ob es richtig ist oder nicht) setze entsprechende Klammern.
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
Kleiner Hinweis:
Code: Alles auswählen
execpath.erase( ProgramPath.find_last_of( "/" ) + 1,
ProgramPath.length() );
Den zweiten Parameter kannst du dir sparen, es wird über ein Default-Parameter der komplette restliche String gelöscht, siehe:
http://www.cplusplus.com/reference/string/string/erase/
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
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 ^^