CopyFile lässt die Anwendung hängen, wie umgehen?

[AlexII]

Hallo,

ich lasse mit CopyFile Dateien kopieren. Während des Kopiervorhangs friert die Anwendung ein. Wie kann man das vermeiden? Den Kopiervorgang in ein extra Thread packen, oder gibt's da auch andere Möglichkeiten?

Danke!

[mkinzler]

Was spricht gegen den Thread? wäre ein weiterer Anwendungsfall für den BackgroundWorker

[himitsu]

Oder CopyFileEx :stupid:

[AlexII]

Zitat:

Zitat von himitsu (Beitrag 1304012)

Oder CopyFileEx :stupid:

Welche uses wird da benötigt?

[Headbucket]

Die ShellApi. Siehe: Klick

Aber ich könnte mir vorstellen, dass es die Funktion in Lazarus gar nicht gibt.
Ein Thread wäre auf jeden Fall der richtige Weg.

Seit Delphi XE7 geht das ja spielend einfach. Wie es bei Lazarus aussieht, kann auch hier nicht beurteilen.

Grüße

[AlexII]

Zitat:

Zitat von Headbucket (Beitrag 1304031)

Die ShellApi. Siehe: Klick

Aber ich könnte mir vorstellen, dass es die Funktion in Lazarus gar nicht gibt.
Ein Thread wäre auf jeden Fall der richtige Weg.

Seit Delphi XE7 geht das ja spielend einfach. Wie es bei Lazarus aussieht, kann auch hier nicht beurteilen.

Grüße

Ja, Lazarus macht da nicht mit... :(

[mm1256]

Zitat:

Zitat von AlexII (Beitrag 1304009)

....oder gibt's da auch andere Möglichkeiten?

Man könnte anstatt "kopieren lassen" selber kopieren und hätte damit so nebenbei auch eine "richtige" Fortschrittsanzeige. Und ab und zu mal ein Application.ProcessMessages würde nichts einfrieren lassen. Und die TimeStamps könnte man auch individuell setzen...und, und, und.

Was ich damit sagen will: Ich hab noch nie CopyFile oder CopyFileEx in einer fertigen Anwendung verwendet.

[himitsu]

Unit Windows? :roll: (zumindestens im Delphi, weil es ja aus der WinAPI kommt)

[AlexII]

Zitat:

Zitat von himitsu (Beitrag 1304040)

Unit Windows? :roll: (zumindestens im Delphi, weil es ja aus der WinAPI kommt)

Windows und ShellApi bringen beide nichts.

[AlexII]

Zitat:

Zitat von mm1256 (Beitrag 1304039)

Zitat:

Zitat von AlexII (Beitrag 1304009)

....oder gibt's da auch andere Möglichkeiten?

Und ab und zu mal ein Application.ProcessMessages würde nichts einfrieren lassen.

Ok... was macht eigentlich

Delphi-Quellcode:

Application.ProcessMessages

und in wiefern lässt es die Anwendung nicht einfrieren?

[mkinzler]

Zitat:

Zitat von AlexII (Beitrag 1304045)

Zitat:

Zitat von mm1256 (Beitrag 1304039)

Zitat:

Zitat von AlexII (Beitrag 1304009)

....oder gibt's da auch andere Möglichkeiten?

Und ab und zu mal ein Application.ProcessMessages würde nichts einfrieren lassen.

Ok... was macht eigentlich

Delphi-Quellcode:

Application.ProcessMessages

und in wiefern lässt es die Anwendung nicht einfrieren?

Gibt Rechenzeit ab, in der Hoffung, das diese durch die UI für eine Aktualisierung genutzt wird.

[AlexII]

Zitat:

Zitat von mkinzler (Beitrag 1304049)

Zitat:

Zitat von AlexII (Beitrag 1304045)

Zitat:

Zitat von mm1256 (Beitrag 1304039)

Zitat:

Zitat von AlexII (Beitrag 1304009)

....oder gibt's da auch andere Möglichkeiten?

Und ab und zu mal ein Application.ProcessMessages würde nichts einfrieren lassen.

Ok... was macht eigentlich

Delphi-Quellcode:

Application.ProcessMessages

und in wiefern lässt es die Anwendung nicht einfrieren?

Gibt Rechenzeit ab, in der Hoffung, das diese durch die UI für eine Aktualisierung genutzt wird.

Dann kann ich praktisch nen Timer setzen und da jede 10 Sekunden

Delphi-Quellcode:

Application.ProcessMessages

aufrufen? Ok, muss ma ausprobieren.

[mkinzler]

Es gibt aber keine Gewähr, dass das richti funktioniert. Besser ist es die Arbeit in einen Hintergrundprozess auzulagern. Der Hauptthread kümmert sich dann nur um die Anzeige des Fortschritts.

[himitsu]

Zitat:

Zitat von AlexII (Beitrag 1304050)

Dann kann ich praktisch nen Timer setzen und da jede 10 Sekunden

Delphi-Quellcode:

Application.ProcessMessages

aufrufen? Ok, muss ma ausprobieren.

Natürlich nicht.

Application.ProcessMessages verarbeitet "jetzt" anstehende Messages in der Message-Queue des Hauptthreads, was so in etwa ganz bestimmt auch in der Hilfe erklärt wird.
Und da Timer-Ereignisse ja bekanntlich über eine Message ausgelöst werden...

[BadenPower]

Zitat:

Zitat von AlexII (Beitrag 1304035)

Ja, Lazarus macht da nicht mit... :(

Dann deklariere die API-Funktion in Lazarus doch selbst.

Code:

function CopyFileEx(lpExistingFileName, lpNewFileName: PChar;
    lpProgressRoutine: Pointer; lpData: Pointer; pbCancel: PBool;
    dwCopyFlags: DWORD): WINBOOL; external 'kernel32' name 'FileCopyExA';

Achtung:
Parameter nicht überprüft.
Eventuell auch noch für 'FileCopyExW' erstellen

[Mavarik]

Zitat:

Zitat von AlexII (Beitrag 1304050)

Dann kann ich praktisch nen Timer setzen und da jede 10 Sekunden

Delphi-Quellcode:

Application.ProcessMessages

aufrufen? Ok, muss ma ausprobieren.

Autsch... Nein...

Warum willst Du den keinen Thread nehmen... Ich überlege jeden Tag: "Kann ich diese 3 Zeilen im Thread ausführen?" Und wenn es dann endlich mal etwas ist, was nicht im Hauptthread laufen muss, freue ich mich ein Schneekönig.

Mavarik

[AlexII]

Zitat:

Zitat von Mavarik (Beitrag 1304071)

Zitat:

Zitat von AlexII (Beitrag 1304050)

Dann kann ich praktisch nen Timer setzen und da jede 10 Sekunden

Delphi-Quellcode:

Application.ProcessMessages

aufrufen? Ok, muss ma ausprobieren.

Autsch... Nein...

Warum willst Du den keinen Thread nehmen... Ich überlege jeden Tag: "Kann ich diese 3 Zeilen im Thread ausführen?" Und wenn es dann endlich mal etwas ist, was nicht im Hauptthread laufen muss, freue ich mich ein Schneekönig.

Mavarik

Ich habe einfach noch nie mit Threads gearbeitet... aber muss wohl jetzt.

[mm1256]

Zitat:

Zitat von AlexII

Ich habe einfach noch nie mit Threads gearbeitet... aber muss wohl jetzt.

Mir geht es ähnlich. Wenn es vermeidbar ist mit Threads zu arbeiten, dann tue ich es. Aber wahrscheinlich ist es wie Radfahren lernen.

Ich hab eine Routine zum Files kopieren, die würde ich auch gerne ab und zu mal in einen Thread packen. Das Problem - aus meiner Sicht - ist die Synchronisation des ProgressBar im Formular. So sieht es momentan aus:

Delphi-Quellcode:

...
type
  TFileCopyMode = (fomAlways, fomIfNewer, fomIfOlder, fomIfSameDate, fomIfSameDateTime);
  TFileCopyCallback  = procedure(ProgressValue: Integer) of object;

...

function MyCopyFile(SourceName, DestName: string;
                    FileCopyMode: TFileCopyMode;
                    Progress: TFileCopyCallback): string;
var
  BytesRead, BytesToRead, P, Percent: Int64;
  SourceStream, DestStream: TFileStream;
  SourceDateTime, DestDateTime: TDateTime;
begin
  Result := '';
  if not FileExists(SourceName) then Exit('Quelldatei <'+SourceName+'> existiert nicht');
  if not FileAge(SourceName, SourceDateTime) then Exit('Quelldatei: TimeStamp lesen fehlgeschlagen');
  BytesRead := 0;
  Percent := 0;
  P := 0;
  // DestName darf auch ein Directory sein...dann den Dateinamen ran hängen
  if DirectoryExists(DestName) // DestName ist ein Directory
  then DestName := IncludeTrailingBackslash(DestName)+ExtractFileName(SourceName);
  if FileExists(DestName) then begin
    if not FileAge(DestName, DestDateTime) then Exit('Zieldatei: TimeStamp lesen fehlgeschlagen');
    case FileCopyMode of
      fomAlways         :;// immer kopieren
      fomIfNewer        : // Die Quelldatei muss neuer als die Zieldatei sein
                          if NOT (SourceDateTime > DestDateTime)
                          then Exit('Die Quelldatei ist nicht neuer als die Zieldatei');
      fomIfOlder        : // Die Quelldatei muss älter als die Zieldatei sein => z.B. Downgrade
                          if NOT (SourceDateTime < DestDateTime)
                          then Exit('Die Quelldatei ist nicht älter als die Zieldatei');
      fomIfSameDate     : // Quell- und Ziel-Datei müssen am selben Tag erstellt worden sein
                          if Trunc(DestDateTime) <> Trunc(SourceDateTime)
                          then Exit('Die Zieldatei ist nicht vom selben Datum als die Quelldatei');
      fomIfSameDateTime : // Das Datum von Quell- und Ziel-Datei muss identisch sein
                          if DestDateTime <> SourceDateTime
                          then Exit('Datum und Zeit von Zieldatei und Quelldatei sind unterschiedlich');
      else raise Exception.Create('Da hat der Programmierer was vergessen');
    end;
  end;
  SourceStream := TFileStream.Create(SourceName,fmOpenRead or fmShareDenyNone);
  DestStream := TFileStream.Create(DestName,fmCreate);
  try
    if @Progress = nil
    then DestStream.CopyFrom(SourceStream,SourceStream.Size) // so geht es am schnellsten
    else begin // mit Fortschrittanzeige
      Progress(0);
      BytesToRead := SizeOf(StreamCopyBuffer);
      if SourceStream.Size < BytesToRead then BytesToRead := SourceStream.Size;
      repeat
        SourceStream.ReadBuffer(StreamCopyBuffer,BytesToRead);
        DestStream.WriteBuffer(StreamCopyBuffer,BytesToRead);
        BytesRead := BytesRead + BytesToRead;
        Percent := (BytesRead * 100) div SourceStream.Size;
        if P <> Percent then begin
          Progress(Integer(Percent));
          P := Percent;
        end;
        if (SourceStream.Size - BytesRead) <= BytesToRead
        then BytesToRead := SourceStream.Size - BytesRead;
      until BytesRead >= SourceStream.Size;
      if BytesRead <> SourceStream.SIZE
      then Result := 'Fehler beim Kopiervorgang:'+#13#10
                     +'Bytes gelesen:' + IntToStr(BytesRead) + #13#10
                     +'Bytes geschrieben:' + IntToStr(SourceStream.SIZE)
      else Progress(100);
    end;
  finally
    SourceStream.Free;
    DestStream.Free;
  end;
  if Result = ''
  then MySetAllFileDates(DestName, SourceDateTime, SourceDateTime, SourceDateTime);
end;

Womit dem TE ja auch geholfen wäre ist die Frage: Wie packt man das in einen Thread?

[BUG]

Zitat:

Zitat von mm1256 (Beitrag 1304148)

Das Problem - aus meiner Sicht - ist die Synchronisation des ProgressBar im Formular.

Mit dem synchronize von TThread ist alles Komplizierte (Aufruf von Methoden im Mainthread über Windows Messages) im Prinzip schon weggekapselt und deine Funktion kann man dank dem Callback auch einfach so verwenden.

Du leitest von TThread ab und rufst dort in execute die Funktion auf. Das Callback ist dabei eine Methode in deiner Thread-Klasse, die das eigentliche Callback (zum Beispiel das Updaten der Progressbar) synchronisiert, dh. im Mainthread, aufruft. Die Parameter für die Funktion kannst du dem Thread im Konstruktor übergeben (der wird noch synchron im Mainthread ausgeführt), für die Rückgabewerte und Exceptions musst du dir noch was überlegen. Ein weiteres synchronisiertes Callback würde sich dafür anbieten.

[mm1256]

Hallo Robert,

danke für den Tipp. Hab's einfach mal probiert....und langsam verliere ich auch die Angst vor den Threads. Weil, es scheint auf Anhieb zu funktionieren.

Für den TE:

Delphi-Quellcode:

unit RuFileCopy;

interface

uses
  System.Classes;

type

  TFileCopyMode = (fomAlways, fomIfNewer, fomIfOlder, fomIfSameDate, fomIfSameDateTime);
  TFileCopyCallback  = procedure(ProgressPercent: Integer) of object;

  TRuFileCopyThread = class(TThread)
  private
    { Private-Deklarationen }
    FCopyResult, FSourceName, FDestName: string;
    FFileCopyMode: TFileCopyMode;
    FFileCopyCallback: TFileCopyCallback;
    procedure ThreadCopyCallback(ProgressPercent: Integer);
  protected
    procedure Execute; override;
  public
    constructor Create(aSourceName, aDestName: string;
                       aFileCopyMode: TFileCopyMode;
                       aFileCopyCallback: TFileCopyCallback);
    procedure ThreadIsReady(Sender: TObject);
  end;

const
  cRuFileCopyReady = 200;

var
  RuFileCopyThreadResult: string = '';

{- Eine Datei kopieren: Direkt-Aufruf -}
function RuFileCopyExecute(aSourceName, aDestName: string;
                           aFileCopyMode: TFileCopyMode;
                           aFileCopyCallback: TFileCopyCallback): string;

{- Eine Datei kopieren: Kopiert die Datei in einem eigenen Thread -}
function RuFileCopyThread(aSourceName, aDestName: string;
                          aFileCopyMode: TFileCopyMode;
                          aFileCopyCallback: TFileCopyCallback): string;

implementation

uses System.SysUtils;

var
  StreamCopyBuffer: array[0..500 * 1024] of byte;
  ProgressPercent: Int64;

{------------------------------------------------------------------------------}
{- Eine Datei kopieren: Direkt-Aufruf oder Aufruf aus dem Thread              -}
{- aSourceName = Name der Quelldatei                                          -}
{- aDestName = Name der ZielDatei, darf auch das Zielverzeichnis sein         -}
{- aFileCopyCallback = ProgressCallback: Übergibt Wert in Prozent von 1-100   -}
{------------------------------------------------------------------------------}
function RuFileCopyExecute(aSourceName, aDestName: string;
                           aFileCopyMode: TFileCopyMode;
                           aFileCopyCallback: TFileCopyCallback): string;
var
  BytesRead, BytesToRead, P: Int64;
  SourceStream, DestStream: TFileStream;
  SourceDateTime, DestDateTime: TDateTime;
begin
  Result := '';
  if not FileExists(aSourceName) then Exit('Quelldatei <'+aSourceName+'> existiert nicht');
  if not FileAge(aSourceName, SourceDateTime) then Exit('Quelldatei: TimeStamp lesen fehlgeschlagen');
  BytesRead := 0;
  ProgressPercent := 0;
  P := 0;
  // DestName darf auch ein Directory sein...dann den Dateinamen ran hängen
  if DirectoryExists(aDestName) // DestName ist ein Directory
  then aDestName := IncludeTrailingBackslash(aDestName)+ExtractFileName(aSourceName);
  if FileExists(aDestName) then begin
    if not FileAge(aDestName, DestDateTime) then Exit('Zieldatei: TimeStamp lesen fehlgeschlagen');
    case aFileCopyMode of
      fomAlways         :;// immer kopieren
      fomIfNewer        : // Die Quelldatei muss neuer als die Zieldatei sein
                          if NOT (SourceDateTime > DestDateTime)
                          then Exit('Die Quelldatei ist nicht neuer als die Zieldatei');
      fomIfOlder        : // Die Quelldatei muss älter als die Zieldatei sein => z.B. Downgrade
                          if NOT (SourceDateTime < DestDateTime)
                          then Exit('Die Quelldatei ist nicht älter als die Zieldatei');
      fomIfSameDate     : // Quell- und Ziel-Datei müssen am selben Tag erstellt worden sein
                          if Trunc(DestDateTime) <> Trunc(SourceDateTime)
                          then Exit('Die Zieldatei ist nicht vom selben Datum als die Quelldatei');
      fomIfSameDateTime : // Das Datum von Quell- und Ziel-Datei muss identisch sein
                          if DestDateTime <> SourceDateTime
                          then Exit('Datum und Zeit von Zieldatei und Quelldatei sind unterschiedlich');
      else raise Exception.Create('Da hat der Programmierer was vergessen');
    end;
  end;
  SourceStream := TFileStream.Create(aSourceName,fmOpenRead or fmShareDenyNone);
  DestStream := TFileStream.Create(aDestName,fmCreate);
  try
    if @aFileCopyCallback = nil
    then DestStream.CopyFrom(SourceStream,SourceStream.Size) // so geht es am schnellsten
    else begin // mit Fortschrittanzeige
      aFileCopyCallback(0);
      BytesToRead := SizeOf(StreamCopyBuffer);
      if SourceStream.Size < BytesToRead then BytesToRead := SourceStream.Size;
      repeat
        SourceStream.ReadBuffer(StreamCopyBuffer,BytesToRead);
        DestStream.WriteBuffer(StreamCopyBuffer,BytesToRead);
        BytesRead := BytesRead + BytesToRead;
        ProgressPercent := (BytesRead * 100) div SourceStream.Size;
        if P <> ProgressPercent then begin
          aFileCopyCallback(Integer(ProgressPercent));
          P := ProgressPercent;
        end;
        if (SourceStream.Size - BytesRead) <= BytesToRead
        then BytesToRead := SourceStream.Size - BytesRead;
      until BytesRead >= SourceStream.Size;
      if BytesRead <> SourceStream.SIZE
      then Result := 'Fehler beim Kopiervorgang:'+#13#10
                     +'Bytes gelesen:' + IntToStr(BytesRead) + #13#10
                     +'Bytes geschrieben:' + IntToStr(SourceStream.SIZE)
      else aFileCopyCallback(100); // Kopiervorgang 100%
    end;
  finally
    SourceStream.Free;
    DestStream.Free;
  end;
end;

function RuFileCopyThread(aSourceName, aDestName: string;
                          aFileCopyMode: TFileCopyMode;
                          aFileCopyCallback: TFileCopyCallback): string;
begin
  with TRuFileCopyThread.Create(aSourceName, aDestName,
                          aFileCopyMode, aFileCopyCallback)
  do begin
    OnTerminate := ThreadIsReady;
    FreeOnTerminate := true;
    Start; // Thread wird gestartet
  end;
end;

{------------------------------------------------------------------------------}
{- TRuFileCopyThread ----------------------------------------------------------}
{------------------------------------------------------------------------------}

procedure TRuFileCopyThread.ThreadCopyCallback(ProgressPercent: Integer);
begin
  Synchronize(procedure
              begin
                FFileCopyCallback(ProgressPercent);
              end);
end;

procedure TRuFileCopyThread.ThreadIsReady(Sender: TObject);
begin
  RuFileCopyThreadResult := FCopyResult;
  Synchronize(procedure
              begin
                FFileCopyCallback(cRuFileCopyReady);
              end);
end;

constructor TRuFileCopyThread.Create(aSourceName, aDestName: string;
  aFileCopyMode: TFileCopyMode; aFileCopyCallback: TFileCopyCallback);
begin
  inherited Create(True); // True = Thread nicht automatisch starten
  FCopyResult := '';
  FSourceName := aSourceName;
  FDestName := aDestName;
  FFileCopyMode := aFileCopyMode;
  FFileCopyCallback := aFileCopyCallback;
  FreeOnTerminate := true;
end;

procedure TRuFileCopyThread.Execute;
begin
  if not Terminated
  then FCopyResult := RuFileCopyExecute(FSourceName, FDestName,
                                        FFileCopyMode, FFileCopyCallback);
end;

end.

// Hauptprogramm

type
  TForm1 = class(TForm)
  ...
  public
    { Public-Deklarationen }
    procedure ShowProgress(ProgressPercent: Integer);
  end;

...

procedure TForm1.ShowProgress(ProgressPercent: Integer);
begin
  ProgressBar1.Position := ProgressPercent;
  if ProgressPercent = cRuFileCopyReady
  then Label4.Caption := 'FERTIG: '+DateTimeToStr(Now)+' Result['+RuFileCopyThreadResult+']';
end;

Man kann während des Kopiervorganges das MainFormular beliebig bewegen, da friert absolut nichts ein und auch der ProgressBar geht schön durch.

ToDo:
- Exception-Behandlung :roll:
- Gibt es eine bessere Lösung für die globale Variable "RuFileCopyThreadResult" und die in der Unit lokalen Variablen StreamCopyBuffer und ProgressPercent [EDIT] Stichwort "ThreadSafe", weil man dann mehrere Dateien gleichzeitig kopieren könnte. Das wär's ;-)

[Sir Rufo]

@mm1256

Also deine FileCopy-Routine ist echt grausam ... wer hat die denn verbrochen? :shock:

Ich wollte die gerade etwas überarbeiten, aber das kommt ja einem neu schreiben gleich.

1. Statt einem
   Delphi-Quellcode:

   String

   als Rückgabe verwende
   Delphi-Quellcode:

   Exceptions

2. Solche Exceptions
   Delphi-Quellcode:

   raise Exception.Create('Da hat der Programmierer was vergessen');

   sind nicht gerade aussagekräftig. Mach es doch richtig:
   
   Delphi-Quellcode:

   case FileCopyMode of
     ...
     else raise ENotImplemented.CreateFmt('FileCopyMode %s nicht implementiert', [GetEnumName( TypeInfo(TFileCopyMode), Ord( FileCopyMode )]);
   end;

   und du bekommst die Fehlermeldung
   
   Zitat:

   FileCopyMode fcmFoo nicht implementiert

3. Statt TStream.ReadBuffer solltest du TStream.Read verwenden, dann bekommst du automatisch die Anzahl der echten gelesenen Bytes zurück. Dann weißt du auch wieviel du schreiben musst
4. Statt einer
   Delphi-Quellcode:

   TFileCopyCallback = procedure(ProgressPercent: Integer) of object;

   nimm eine
   Delphi-Quellcode:

   TFileCopyCallback = reference to procedure(ProgressPercent: Integer);

   (optional, weil flexibler)

Wenn du das hast, dann kannst du diese Kopieraktion vom http://www.delphipraxis.net/185335-b...ab-xe2%5D.html aus starten und alles wird gut.

Delphi-Quellcode:

procedure TFoo.FileCopyWorker_DoWork(Sender: TObject; e: TDoWorkEventArgs );
var
  FileCopyArg : TFileCopyArg;
begin
  FileCopyArg := e.Argument.AsType<TFileCopyArg>;
  MyCopyFile( 
    FileCopyArg.SourceName, 
    FileCopyArg.DestName, 
    FileCopyArg.FileCopyMode,
    (Sender as TBackgroundWorker).ReportProgress );
end;

procedure TFoo.FileCopyWorker_ProgressChanged( Sender : TObject; e: TProgressChangedEventArgs );
begin
  ProgressBar1.Position := e.Value;
end;

procedure TFoo.FileCopyWorker_RunWorkerCompleted( Sender: TObject; e: TRunWorkerCompletedEventArgs );
begin
  if Assigned( e.Error ) then
    ShowMessage( e.Error.ToString() )
  else if e.Cancelled then
    ShowMessage( 'Du hast das abgebrochen!' )
  else
    ShowMessage( 'Ich habe fertig!' );
end;

Das ist eigentlich alles und du kannst die originale Routine verwenden.

PS Das Wichtigste hatte ich vergessen:
Du greifst immer wieder auf

Delphi-Quellcode:

SourceStream.Size

zu, und das macht deine Routine mit Progress-Callback extremst langsam. Denn bei jedem Aufruf wandert der Stream an das Ende liest die Position und kehrt zur vorherigen Position zurück!

Lies den Wert am Anfang einmal aus und merk dir diesen, dann läuft auch die Routine mit dem Progress sehr schnell.

[mm1256]

Zitat:

Zitat von Sir Rufo (Beitrag 1304177)

Also deine FileCopy-Routine ist echt grausam ... wer hat die denn verbrochen? :shock:

Dieser Verbrecher antwortet gerade :!:

Ich freue mich immer sehr, wenn Verbesserungsvorschläge dazu beitragen, etwas "zu verbessern". Darum habe ich mir deine Vorschläge mal zur Brust genommen:

Zitat:

Zitat von Sir Rufo (Beitrag 1304177)

[*]Statt einem

Delphi-Quellcode:

String

als Rückgabe verwende

Delphi-Quellcode:

Exceptions

Mittlerweile kenne ich deinen Standpunkt zur Exception-Behandlung, aber in diesem Fall bringen mir Exceptions anstelle des String-Rückgabewertes (und so hast du das wohl gemeint) überhaupt nicht weiter. Welche Exception soll beispielsweise geworfen werden, wenn die Zieldatei neuer ist? Da ist meiner Meinung nach ein schlichter Rückgabewert geeigneter. Der User soll ja bei Bedarf lediglich darüber informiert werden, was los ist.

Zitat:

Zitat von Sir Rufo (Beitrag 1304177)

[*]Solche Exceptions

Delphi-Quellcode:

raise Exception.Create('Da hat der Programmierer was vergessen');

sind nicht gerade aussagekräftig. Mach es doch richtig:

Delphi-Quellcode:

case FileCopyMode of
  ...
  else raise ENotImplemented.CreateFmt('FileCopyMode %s nicht implementiert', [GetEnumName( TypeInfo(TFileCopyMode), Ord( FileCopyMode )]);
end;

Gerade um das "richtig machen geht es hier. Diese Exception wird im Normalfall nur im Entwicklungsmodus stattfinden. Der User wird diese niemals zu Gesicht bekommen, denn wenn das passiert, hat der Programmierer bereits Bockmist gebaut und sein Werk nicht mal ansatzweise getestet.

Zitat:

Zitat von Sir Rufo (Beitrag 1304177)

PS Das Wichtigste hatte ich vergessen:
Du greifst immer wieder auf

Delphi-Quellcode:

SourceStream.Size

zu, und das macht deine Routine mit Progress-Callback extremst langsam. Denn bei jedem Aufruf wandert der Stream an das Ende liest die Position und kehrt zur vorherigen Position zurück!

Lies den Wert am Anfang einmal aus und merk dir diesen, dann läuft auch die Routine mit dem Progress sehr schnell.

Das mit dem Getter der Filesize habe ich nicht gewusst. Aber, ich habe es jetzt mal geändert (bin ja nicht beratungsresistent) und mit einer 3,5 GB großen Datei ausprobiert und die Zeiten verglichen. Die Formulierung "extremst langsam" ist total überzogen. Der Unterschied liegt bei dieser 3,5 GB-Datei im Bereich von einigen Millisekunden.

Wenn also Performance raus zu holen ist, dann nicht an dieser Stelle.

[Sir Rufo]

• Zu
  Delphi-Quellcode:

  Exception

  :
  
  Bei einer Exception wird der Benutzer doch informiert ... es poppt eine MessageBox mit der Meldung auf, ganz von alleine, nur weil du eine Exception wirfst.
  
  Und wenn du mit dem Argument
  Delphi-Quellcode:

  FileCopyMode

  eine Bedingung festlegst, die dann nicht zutrifft, dann ist das eine Ausnahme. Also Exception werfen und gut.
  
  Die Exception könntest du als
  
  Delphi-Quellcode:

  type
    EFileCopyModeMismatchException = class(Exception);

  deklarieren und entsprechend werfen. Dann kannst du im nachhinein sogar noch speziell darauf reagieren -> Dialog und den Benutzer fragen, ob er evtl. eine anderen
  Delphi-Quellcode:

  FileCopyMode

  haben möchte.
  
  Und da eine Exception auch nur eine Klasse ist, kannst du da sogar noch alle möglichen Informationen hineinpacken (z.B. den FileCopyMode, der diese Exception verursacht hat).
  
• Zu
  Delphi-Quellcode:

  ENotImplemented

  :
  
  Wenn dich dein Kunde anruft und dir mitteilt, dass da so eine Fehlermeldung aufgetaucht ist, bei welcher Fehlermeldung wüsstest du sofort worum es geht? - Eben, sehe ich auch so ;)
  
  Ach ja, Fehler passieren immer wieder, auch beim Testen ...
  
• Zu
  Delphi-Quellcode:

  TStream.Size

  :
  
  Wie stark das bremst hängt natürlich stark von deiner Hardware und wie stark fragmentiert die Datei auf der Platte vorliegt. Im Idealfall sollte man davon nicht wirklich viel merken, im Worst Case kannst du in der Zeit halb Asien neu bevölkern ;)

[Dejan Vu]

Zitat:

Zitat von mm1256 (Beitrag 1304186)

+...aber in diesem Fall bringen mir Exceptions anstelle des String-Rückgabewertes überhaupt nicht weiter.

Und wie willst Du deine Rückgabefehlermeldungszeichenkette auswerten? Also, woher weißt Du, welcher Fehler genau aufgetreten ist? Ach, das ist Dir egal? Man kann ja streiten, ob Exceptions oder Rückgabewerte besser sind, aber wenn schon Rückgabewerte, dann wenigstens numerisch, oder als Enum.

Stell Dir mal vor, dein Programm soll mal im Ausland laufen...

Übrigens ist es keine Tugend, alles in eine Routine zu quetschen. Das ist sowas von gestern, ach was sag ich, vorvorvorgestern... Definier doch einfach eine Klasse, führe einfache kleine Methoden ein und schreibe das Ganze, das es verständlich ist. "Clean Code" nennt sich das. Probiere es mal aus.

[BUG]

Zitat:

Zitat von Dejan Vu (Beitrag 1304193)

Übrigens ist es keine Tugend, alles in eine Routine zu quetschen. Das ist sowas von gestern, ach was sag ich, vorvorvorgestern... Definier doch einfach eine Klasse, führe einfache kleine Methoden ein und schreibe das Ganze, das es verständlich ist. "Clean Code" nennt sich das. Probiere es mal aus.

Prinzipiell bin ich deiner Meinung, aber bei dieser "Begründung" ziehst du niemanden aus dem Schatten der seitenlangen Prozeduren in säubernde Licht des blütenreinen Programmierens :stupid:

Die Reaktionen sind imho leicht übertrieben (bis auf die zur Fehlerbehandlung :wink:). Es gibt deutlich schlimmeren Code.
Der Hinweis auf den Workerpool ist für diese Aufgabe zwar legitim, man sollte das imho aber auch mal zu Fuß gemacht haben, damit man eine Vorstellung davon hat, was bei so einer asynchronen Ausführung so passiert.

Ein Fehler ist mir ins Auge gefallen: Der RuFileCopyExecute wird in Execute das falsche Callback übergeben. Eigentlich willst du ja die synchronisierte Variante.

Das CreateSuspended = true und das Konstrukt mit dem with kannst du dir sparen, wenn du die Eigenschaften, die du im with setzt, direkt im Konstruktor setzt.
OnTerminate wird im Hauptthread ausgeführt, das synchronize solltest du dir an der Stelle sparen; oder du lässt onTerminate leer und rufst

Delphi-Quellcode:

ThreadCopyCallback(cRuFileCopyReady);

direkt am Ende von Execute auf.

[Dejan Vu]

Zitat:

Zitat von BUG (Beitrag 1304201)

Prinzipiell bin ich deiner Meinung, aber bei dieser "Begründung" ziehst du niemanden aus dem Schatten der seitenlangen Prozeduren in säubernde Licht des blütenreinen Programmierens :stupid:

:oops: Ash on my main.

[mm1256]

Hallo,

ich habe den Eindruck, dass ihr mit den Exceptions jetzt total am Ziel vorbei argumentiert. Fangen wir doch mal ganz von vorne an. Wozu bzw. in welchen Anwendungsfällen verwende ich überhaupt eine "eigene" Kopier-Routine? Immer dann, wenn die normale WinApi oder was auch immer nicht ausreicht, oder Probleme verursacht, oder die Funktionalität nicht ausreicht.

In diesem Fall geht es darum, dass dem TE die Mainform einfriert. Also gibt es zwei grundsätzliche Lösungsmöglichkeiten: Separater Thread und/oder eigene Kopier-Routine. Soweit so gut, nun mal zur praktischen Anwendung. Wenn ich eine oder mehrere Dateien kopiere, dann brauche ich eine Rückmeldung, wenn dabei etwas schief geht, z.B. Festplatte ist voll, Ziel-Datenträger ist schreibgeschützt...was auch immer. Aber eine Exception werfen, weil z.B. die Zieldatei neuer ist als die Quelldatei? Sorry, aber das ist meiner Meinung nach ganz schlechter Programmierstil. Wie will ich in dem Fall eine ich nenne sie mal fatale Exception (z.B. Datenträger ist voll) von einer ganz normalen Meldung "Zieldatei ist neuer" unterscheiden? Hinzu kommt, in manchen Situationen will ich eine Meldung vielleicht gar nicht anzeigen, sondern ganz einfach ältere Dateien durch neuere ersetzen. Wie sieht das dann in eurem Quellcode aus?

Delphi-Quellcode:

try
  Dateien kopieren...
except
  // hier den ganzen Rotz auswerten, weil, die ganz normalen User-Messages - wenn 
  // ich sie denn überhaupt dem User anzeigen möchte, landen auch hier.
end;

Das kann nicht wirklich euer Ernst sein. So würde ich das machen:

Delphi-Quellcode:

try
  Dateien kopieren...
  Bei Bedarf dem User entsprechende Messages zeigen, oder bei vielen Dateien alle Messages 
  in eine Liste und am Schluss des Kopiervorganges anzeigen
except
  // hier die "echten" Exceptions auswerten, z.B. Datenträger ist voll
end;

Jeder kann das halten wie er will. Für mich sind Exceptions "Ausnahmen" und wenn ich eine Ausnahme nicht behandeln kann, nach eurer Logik z.B. die Ausnahme "Die Zieldatei ist neuer als die Quelldatei", wozu muss ich sie dann überhaupt erst mal werfen? Das widerspricht dem Grundprinzip der Exceptionbehandlung. Ihr verwechselt da ganz einfach zwei grundverschiedene Situationen: Ein wahlweise nach Bedarf für den User anzuzeigendes Ergebnis eines Kopiervorganges und einer Exception die bei diesem Vorgang aufgetreten ist.

Zitat:

Zitat von Dejan Vu

Und wie willst Du deine Rückgabefehlermeldungszeichenkette auswerten? Also, woher weißt Du, welcher Fehler genau aufgetreten ist? Ach, das ist Dir egal?

Das ist genau der Punkt wo du total daneben liegst. Wenn ich genau wissen will, was bei der Kopieraktion passiert ist, dann ist beispielsweise ein Enum die richtige Wahl. Doch, das will und muss ich in diesem Fall doch gar nicht.

Wenn ich der Meinung bin, dass dem User eine Meldung angezeigt werden muss, dann zeige ich sie ihm. Wenn ich für den Fall, dass der Kopiervorgang ohne Usermeldung ablaufen soll, dem User etwas zeigen will oder muss, dann ist es eine "echte" Exception, z.B. "deine Platte ist voll".

Zitat:

Zitat von Dejan Vu

Stell Dir mal vor, dein Programm soll mal im Ausland laufen...

Muss ich mir das vorstellen, und ist diese Aufgabenstellung Bestandteil dieses Threads? Nein. Also wozu dieses "provokannte Argument". Ach ja, die Antwort gibst du ja selber: Es gibt Besserwisser....

Zitat:

Zitat von BUG

Das CreateSuspended = true und das Konstrukt mit dem with kannst du dir sparen, wenn du die Eigenschaften, die du im with setzt, direkt im Konstruktor setzt.
OnTerminate wird im Hauptthread ausgeführt, das synchronize solltest du dir an der Stelle sparen; oder du lässt onTerminate leer und rufst ThreadCopyCallback(cRuFileCopyReady); direkt am Ende von Execute auf.

Vielen Dank für den - aus meiner Sicht leider einzigen - konstruktiven Hinweis.

[p80286]

Zitat:

Zitat von mm1256 (Beitrag 1304218)

Wenn ich der Meinung bin, dass dem User eine Meldung angezeigt werden muss, dann zeige ich sie ihm. Wenn ich für den Fall, dass der Kopiervorgang ohne Usermeldung ablaufen soll, dem User etwas zeigen will oder muss, dann ist es eine "echte" Exception, z.B. "deine Platte ist voll".

Kann ich gut nachvollziehen, wobei es drei Arten von Ausnahmen/Fehlern gibt, die an die man vorher denkt und die man im Vorfeld vermeiden kann, die um die sich der Benutzer kümmern muß, und die an die man nicht gedacht hat.(und dafür sind exceptions wirklich was feines)

Zitat:

Zitat von mm1256 (Beitrag 1304218)

Zitat:

Zitat von Dejan Vu

Stell Dir mal vor, dein Programm soll mal im Ausland laufen...

Muss ich mir das vorstellen, und ist diese Aufgabenstellung Bestandteil dieses Threads? Nein. Also wozu dieses "provokannte Argument". Ach ja, die Antwort gibst du ja selber: Es gibt Besserwisser....

Ich habe die Erfahrung gemacht, daß ein QuicknDirty Tool eine "internationale" Karriere gemacht hat. Darum Wirf den Einwand nicht zu weit weg. Zumindest rudimentär sollte man die Möglichkeit in Betracht ziehen.

Gruß
K-H

[Sir Rufo]

Zu Exceptions gibt es hier mehrere Themen und Beiträge und es wurde schon erschöpfend behandelt.

Und ich finde es wesentlich einfacher auf einen speziellen Exception-Typ zu reagieren, als einen String-Rückgabewert zu parsen um herauszufinden, was denn jetzt eigentlich los gewesen ist, und wie ich da am sinnvollsten reagieren könnte.

Und wenn dann noch unterschiedliche Sprachen mit ins Spiel kommen, dann brauche ich vor dem Parsen sogar noch G**gle-Translate ...

[mm1256]

Zitat:

Zitat von Sir Rufo (Beitrag 1304226)

Zu Exceptions gibt es hier mehrere Themen und Beiträge und es wurde schon erschöpfend behandelt.

Und ich finde es wesentlich einfacher auf einen speziellen Exception-Typ zu reagieren, als einen String-Rückgabewert zu parsen um herauszufinden, was denn jetzt eigentlich los gewesen ist, und wie ich da am sinnvollsten reagieren könnte.

Und wenn dann noch unterschiedliche Sprachen mit ins Spiel kommen, dann brauche ich vor dem Parsen sogar noch G**gle-Translate ...

Sorry Sir Rufo, du hast es immer noch nicht verstanden. Ich parse keinen Rückgabe-String. Wenn ich - oder jemand anders - das für nötig empfinden würde, dann kann er das ja so machen wie er will. Ich würde dann einen ENUM nehmen. In diesem Fall geht es aber ausschließlich darum, den User bei Bedarf über einen Sachverhalt, z.B. "Datei ist neuer", zu informieren. Ob der User darauf reagieren will, und wie, das kann bzw. muss er dann schon selber entscheiden. Das Thema "Exceptionbehandlung" hat damit überhaupt nichts zu tun. Ich weiß, hart für dich als "Exception-Spezialisten", aber manchmal muss man(n) eben auch mal ganz einfach die Aufgabenstellung sehen.

[OT]

Zitat:

Zitat von p80286 (Beitrag 1304222)

Ich habe die Erfahrung gemacht, daß ein QuicknDirty Tool eine "internationale" Karriere gemacht hat. Darum Wirf den Einwand nicht zu weit weg. Zumindest rudimentär sollte man die Möglichkeit in Betracht ziehen.

Gruß
K-H

"rudimentär" = Ja sicher. Wenn's denn mal wieder so sein sollte greife ich auf mein noch aus den 90-er Jahren stammendes Tool für die Internationalisierung zurück. Damals hatte ich eine KFZ-Software programmiert (die es übrigens immer noch gibt) mit welcher man zur Laufzeit auf Französisch, Englisch, Italienisch, Tschechisch und Polnisch umschalten konnte.
[/OT]