3. directory iterator

directory iterator

  • G

    so, ich hab eure ratschläge mal versucht zu realisieren und die klasse überarbeitet.
    also ich finde sie sieht gleich viel schöner aus und die ersten test haben auch funktioniert 😉

    Bitte gebt mir eine kleine rückmeldung, ob ihr das so gemeint habt und ob ich so für den anfang zufrieden sein kann.

    boost::iterator_facade

    habe ich nicht genommen, weil wenn ich boost benutzen würde wer der sinn des selberschreibens weg und ich könnte gleich boost::filesystem::directory_iterator nehmen 🤡

    (nurnoch ~100 zeilen)

    #include "..\compileconfig.h"

#ifndef DELTA_FILE_directory_iterator_H
#define DELTA_FILE_directory_iterator_H

#include <string>
#include <memory>

//#include <cstdio>
#include <Windows.h>

#include "..\core\include.h"
#include "path.h"

namespace delta{
namespace file{

	class directory_iterator{
	public:
		typedef std::input_iterator_tag iterator_category; // input- und single pass iterator 
		typedef file::path value_type; 
		typedef std::ptrdiff_t difference_type; 
		typedef const file::path* pointer; 
		typedef const file::path& reference; // const-iterator 

		static directory_iterator end(){
			directory_iterator ret;
			ret.current.clear();
			return ret;
		}

	public:
		// construction
		directory_iterator() // end-iterator   
			: find_data() 
			, hfile() 
			, current()
		{}
		explicit directory_iterator(const file::path& p)
			: hfile( FindFirstFile( (p.wstring()+L"*").c_str() , &find_data ), &FindClose )
			, current( hfile.get() != INVALID_HANDLE_VALUE? find_data.cFileName: TEXT("") )
		{}

	public:
		// operators
		directory_iterator& operator ++ (){
			forward();
			return *this;
		}
		directory_iterator operator ++(int) { 
			directory_iterator tmp = *this;
			forward();
			return tmp;
		}
		directory_iterator& operator += (int num){
			if(num < 0) assert("delta::file::directory_iterator: no backward traversing");

			while(num--) forward();
			return *this;
		}
		directory_iterator operator + (int num) const{ 
			directory_iterator tmp = *this;
			return tmp += num;
		}

		bool operator == (const directory_iterator& other) const{
			return (current == other.current);
		}

		bool operator != (const directory_iterator& other) const{
			return (current != other.current);
		}

	  #if defined (_MSC_VER) && (_MSC_VER < 1300)
		#pragma warning(disable:4284) // infix notation problem when using iterator operator ->
	  #endif

		const file::path & operator * () { return current; }
		const file::path * operator ->() { return &current; }

	private:
		// step forward to the next file
		void forward(){
			if(FindNextFile(hfile.get(), &find_data))
				current.assign(find_data.cFileName);
			else
				current.clear();
		}

	private:
		typedef std::shared_ptr< std::remove_pointer< HANDLE >::type > shared_handle;

		WIN32_FIND_DATA find_data; 
		shared_handle hfile; 
		file::path current; // current filepath 
	};

};// ns: file
};// ns: delta

#endif DELTA_FILE_directory_iterator_H
    0
  • W

    Hallo Jannik,

    Zeile 29 sollte überflüssig sein. Ich kenne die Implementierung von file::path zwar nicht, aber der Defaultkonstruktor sollte einen 'gültigen' leeren Pfad erzeugen. Dann wird aber auch die statische Methode end() überflüssig, es sei denn wegen 'syntactic sugar'.

    Ist nur formal, aber in Zeile 41 fehlt find_data in der Initialisierungsliste. Es gibt statisch Test (Lint), die das an-meckern würden.

    Die Operatoren operator+ und operator+= sind unnötig, da sie mit advance realisiert werden können. Sie implizieren dem Anwender lediglich, dass operator+ mit O(1)-Komplexität implementiert ist, was aber nicht der Fall ist (nicht sein kann). Das lädt nur zu einem Fehlgebrauch ein.
    Wegen so was hatte ich vor Jahren mal einen dicken Laufzeitfehler in ein Programm eingebaut - ich brauchte mehrere Test beim Kunden, um die Ursache zu ermitteln.

    operator*() und operator->() sollten und können const sein.

    Wenn zwei Iteratoren gleich sind, wenn der Filename gleich ist, so bedeutet dass, dass auch zwei Iteratoren aus unterschiedlichen Directorys gleich sein können. Den operator!= sollte man als !(*this == other) implementieren, um Redundanzen zu vermeiden.

    Gruß
    Werner

    0
  • G

    Hallo Jannik,

    oh, da war aber einer aufmerksam 🤡

    'syntactic sugar'

    cooler begriff 😃

    habe alle fehler bis auf einen ausgebessert 😉

    Die Operatoren operator+ und operator+= sind unnötig, da sie mit advance realisiert werden können. Sie implizieren dem Anwender lediglich, dass operator+ mit O(1)-Komplexität implementiert ist, was aber nicht der Fall ist (nicht sein kann). Das lädt nur zu einem Fehlgebrauch ein.
    Wegen so was hatte ich vor Jahren mal einen dicken Laufzeitfehler in ein Programm eingebaut - ich brauchte mehrere Test beim Kunden, um die Ursache zu ermitteln.

    verstehe das problem, aber da ich std::advance noch nie benutzt habe weiß ich nicht, wie ich es benutzen kann 😕

    0
  • W

    Hallo Jannik,

    gamer8o4 schrieb:

    verstehe das problem, aber da ich std::advance noch nie benutzt habe weiß ich nicht, wie ich es benutzen kann 😕

    Dafür hatte ich Dir in meinem Posting den Link angegeben. Da gibt es auch ein Beispiel.

    oder so:

    int main()
{
    using namespace std;
    file::path dir( TEXT("C:\\*") );
    int anz = distance( directory_iterator( dir ), directory_iterator() );
    directory_iterator i( dir );
    advance( i, anz/2 ); // inkrementiere den Iterator bis zur Hälfte der Files
    copy( i, directory_iterator(), ostream_iterator< directory_iterator::value_type, wchar_t >( wcout << "Die zweite Haelfte:\n", L"\n" ) );

    return 0;
}

    Gruß & Gute Nacht
    Werner

    0
  • ?

    Werner Salomon schrieb:

    Sowas nennt man eine 'shared resource'. Und da das so oft vorkommt, muss man das nicht selber coden, denn es gibt in C++11 eine Klasse, die das kann. Das ist der std::shared_ptr<>. Dem (Smart-)Pointer kann man mitgeben, was er tun soll, wenn er bzw. die letzte seiner Kopien gelöscht wird. Hier wäre das die Funktion FindClose .

    Sowas nennt man Move-Only-Type. Man deaktiviert den Kopierkonstruktor/Zuweisungsoperator und schreibt dafür einen Move-Konstruktor und Move-Zuweisungsoperator. Wer die Ressource aus unerfindlichen Gründen sharen will (in deinem Beispiel würde eine Referenz auch genügen), muss das halt mit shared_ptr<directory_iterator> explizit hinschreiben.

    0
  • G

    in deinem Beispiel würde eine Referenz auch genügen

    ein sehr interessanter einfall!! 😮
    ich glaub das schreibe ich mal direkt um, ich mag die idee mit der referenz und es erscheint mir sicherer und schneller also... 🙂

    0
  • W

    Hallo gamer8o4,

    gamer8o4 schrieb:

    in deinem Beispiel würde eine Referenz auch genügen

    ein sehr interessanter einfall!! 😮 ... und es erscheint mir sicherer und schneller

    Ja - interessanter, schneller, verführerischer - quasi die dunkle Seite von C++.

    Ein directory_iterator ist zunächst einmal ein Iterator. Und ein Iterator muss - um dem C++ Standard zu genügen - 'CopyConstructible' sein. Steht ganz vorn bei dem Anforderungen an einen Iterator. Ist er das nicht, so kannst Du noch nicht mal einen std::copy damit aufrufen. Probiere es aus - mache den Kopierkonstruktor privat - Dein Compiler wird das gar nicht übersetzen!

    directory_iterator i = ....;
    copy( i, directory_iterator::end(), ostream_iterator<...>(...) );

    Praktisch alle Algorithmen verlangen das, wenn auch in dem einen oder anderen Fall der jeweiligen Implementierung ein moveable ausreichen würde. Und was bitte schön sollte man mit eine shared_ptr<Iterator> anfangen?

    Gruß
    Werner

    0
  • G

    also nicht mit referenzen machen? JA oder NEIN?

    0
  • N

    NEIN

    0
  • Mod

    gamer8o4 schrieb:

    also nicht mit referenzen machen? JA oder NEIN?

    Doppelte Verneinung in schriftlicher Kommunikation ist böse. Denkst du nicht auch, dass das nicht so ist? Ja oder Nein?

    0
  • N

    Oh stimmt ja.
    Fail.
    Ich meine:
    JA

    0
  • G

    also JA, okay ich ändere es 😃

    0
Anmelden zum Antworten