Thread mit Fortschrittsanzeige

[Nino]

Hallo,

ich habe mit Threads kaum Erfahrung, und würde mich daher freuen, wenn ihr Euch folgendes "Code-Gerüst" mal ansehen würdet.
Da man ja in Threads kein GUI-Zeugs machen soll, habe ich für die Aktualisierung der Fortschrittsanzeige PostEvent() benutzt.
Sind Fehler in dem Code, oder lässt sich sonst etwas verbessern? Ich bin für jeden Hinweis dankbar.

Code: Alles auswählen

EnableExplicit

; Windows
Enumeration
   #winMain
EndEnumeration

; Gadgets
Enumeration
   #btnDoIt
   #btnQuit
EndEnumeration

; Statusbar
Enumeration
   #status
EndEnumeration

; Custom events
Enumeration #PB_Event_FirstCustomValue
   #Event_Progress
   #Event_ThreadFinished
EndEnumeration   

Global.i ThreadActive = #False


Procedure CopyFiles(n)
   Protected.i i
   
   For i = 1 To n
      If ThreadActive = #False
         Break
      EndIf   
      
      Delay(1000)        ; oder Dateien kopieren oder ...
      PostEvent(#Event_Progress)
   Next
   
   PostEvent(#Event_ThreadFinished)
EndProcedure


#Acton$ = "Los geht's"

Procedure Main()
   Protected.i n, stepSize, done, event, thread
   
   If OpenWindow(#winMain, #PB_Ignore, #PB_Ignore, 500, 300, "Thread mit Fortschrittsanzeige") = 0
      MessageRequester("Schwerer Fehler", "Hauptfenster kann nicht geöffnet werden.")
      End
   EndIf   
   
   ButtonGadget(#btnDoIt, 240, 230, 110, 35, #Acton$)
   ButtonGadget(#btnQuit, 385, 230, 100, 35, "Beenden")
   If CreateStatusBar(#status, WindowID(#winMain))
      AddStatusBarField(90)
      AddStatusBarField(#PB_Ignore)
   EndIf
   
   n = 20
   stepSize = 100 / n
   
   Repeat
      Select WaitWindowEvent()
         Case #PB_Event_Gadget
            Select EventGadget()
               Case #btnDoIt
                  If ThreadActive = #False
                     done = 0
                     StatusBarText(#status, 0, "")
                     StatusBarProgress(#status, 1, done)
                     SetGadgetText(#btnDoIt, "Abbrechen")
                     DisableGadget(#btnQuit, #True)
                     ThreadActive = #True
                     thread = CreateThread(@CopyFiles(), n)    ; n Dateien kopieren
                  Else
                     ThreadActive = #False
                  EndIf 
                  
               Case #btnQuit
                  Break
            EndSelect
            
         Case #PB_Event_CloseWindow
            If ThreadActive = #False
               Break
            EndIf
            
         Case #Event_Progress
            done + stepSize
            StatusBarText(#status, 0, Str(done) + "%")
            StatusBarProgress(#status, 1, done)
            
         Case #Event_ThreadFinished
            If ThreadActive = #True
               StatusBarText(#status, 0, "Fertig")
               ThreadActive = #False
            Else   
               StatusBarText(#status, 0, "Abgebrochen")
            EndIf   
            SetGadgetText(#btnDoIt, #Acton$)
            DisableGadget(#btnQuit, #False)
      EndSelect
   ForEver
   
   CloseWindow(#WinMain)
EndProcedure


Main()

[ts-soft]

Zum Progress anzeigen wäre BindEvent() besser, da dann der Fortschritt auch angezeigt wird, wenn das Fenster bewegt wird.

Code: Alles auswählen

EnableExplicit

; Windows
Enumeration
  #winMain
EndEnumeration

; Gadgets
Enumeration
  #btnDoIt
  #btnQuit
EndEnumeration

; Statusbar
Enumeration
  #status
EndEnumeration

; Custom events
Enumeration #PB_Event_FirstCustomValue
  #Event_Progress
  #Event_ThreadFinished
EndEnumeration   

Global.i ThreadActive = #False
Define done, stepSize

Procedure CopyFiles(n)
  Protected.i i
  
  For i = 1 To n
    If ThreadActive = #False
      Break
    EndIf   
    
    Delay(1000)        ; oder Dateien kopieren oder ...
    PostEvent(#Event_Progress)
  Next
  
  PostEvent(#Event_ThreadFinished)
EndProcedure

Procedure Event_Progress()
  Shared done, stepSize
  
  done + stepSize
  StatusBarText(#status, 0, Str(done) + "%")
  StatusBarProgress(#status, 1, done)
EndProcedure

#Acton$ = "Los geht's"

Procedure Main()
  Protected.i n, event, thread
  Shared done, stepSize
  
  If OpenWindow(#winMain, #PB_Ignore, #PB_Ignore, 500, 300, "Thread mit Fortschrittsanzeige") = 0
    MessageRequester("Schwerer Fehler", "Hauptfenster kann nicht geöffnet werden.")
    End
  EndIf   
  
  ButtonGadget(#btnDoIt, 240, 230, 110, 35, #Acton$)
  ButtonGadget(#btnQuit, 385, 230, 100, 35, "Beenden")
  If CreateStatusBar(#status, WindowID(#winMain))
    AddStatusBarField(90)
    AddStatusBarField(#PB_Ignore)
  EndIf
  
  BindEvent(#Event_Progress, @Event_Progress())
  
  n = 20
  stepSize = 100 / n
  
  Repeat
    Select WaitWindowEvent()
      Case #PB_Event_Gadget
        Select EventGadget()
          Case #btnDoIt
            If ThreadActive = #False
              done = 0
              StatusBarText(#status, 0, "")
              StatusBarProgress(#status, 1, done)
              SetGadgetText(#btnDoIt, "Abbrechen")
              DisableGadget(#btnQuit, #True)
              ThreadActive = #True
              thread = CreateThread(@CopyFiles(), n)    ; n Dateien kopieren
            Else
              ThreadActive = #False
            EndIf
            
          Case #btnQuit
            Break
        EndSelect
        
      Case #PB_Event_CloseWindow
        If ThreadActive = #False
          Break
        EndIf
        
;       Case #Event_Progress
;         done + stepSize
;         StatusBarText(#status, 0, Str(done) + "%")
;         StatusBarProgress(#status, 1, done)
        
      Case #Event_ThreadFinished
        If ThreadActive = #True
          StatusBarText(#status, 0, "Fertig")
          ThreadActive = #False
        Else   
          StatusBarText(#status, 0, "Abgebrochen")
        EndIf   
        SetGadgetText(#btnDoIt, #Acton$)
        DisableGadget(#btnQuit, #False)
    EndSelect
  ForEver
  
  CloseWindow(#WinMain)
EndProcedure


Main() 

Ansonsten sind noch Verbesserungen in der Abhandlung von Abbrechen usw. möglich. Da hab ich aber gerade
keine Lust Code für zu schreiben, das wirste hoffentlich selber schaffen, ansonsten bin ich hier ja nicht alleine

Gruß
Thomas

[Nino]

Vielen Dank, Thomas!

Einen Mutex brauche ich da nicht, oder?

[GPI]

nö, beide Threads sind doch unabhängig, oder? Du brauchst einen Mutex, wenn du auf das gleiche Element zugreifen willst. Bspw. wenn du eine Log-Datei hast, wäre es ungünstig, wenn Thread und Mainprogramm gleichzeitig irgendwelche Daten ins log zu schreiben. Nachrichten verschicken (PostEvent macht das) ist da ungefährlich und genau die richtige Methode.

[ts-soft]

nö, beide Threads sind doch unabhängig, oder?

Naja, ich sehe nur einen Thread , das andere ist ein Callback. Ansonsten stimmt's schon.

Mutex wird in diesem Stadium noch nicht benötigt, aber ich denke, wenn der Code etwas vollständiger wird,
ist er erforderlich, genauso wie Threadssafe. Aber im Moment ist da noch nichts abzusichern, es wird ja nur
Delay(1000) kopiert

[Nino]

Danke für Eure Antworten.

Mutex wird in diesem Stadium noch nicht benötigt, aber ich denke, wenn der Code etwas vollständiger wird,
ist er erforderlich, genauso wie Threadssafe. Aber im Moment ist da noch nichts abzusichern, es wird ja nur
Delay(1000) kopiert

Aber später sollen schon echte Dateien kopiert werden.
Ist dann ein Mutex nötig?

[ts-soft]

Kommt drauf an, aber im allg. schon!
Wenn Du z.B. eine Liste mit Dateien übergibst, muß diese abgesichert werden. Aber ohne Code kann ich da nicht viel zu sagen.

[GPI]

nö, beide Threads sind doch unabhängig, oder?

Naja, ich sehe nur einen Thread , das andere ist ein Callback. Ansonsten stimmt's schon.

Ich seh das "Hauptprogram" auch als Thread an

Aber um zum Thema ein bischen zurückzukommen, kleiner Auszug aus einen meiner Programme:

Code: Alles auswählen

Structure files
  from.s
  dest.s
EndStructure


Procedure.s GetLastErrorMsg(); - Return the Last Error Message
  Define buffer.i
  Define result$
  FormatMessage_(#FORMAT_MESSAGE_ALLOCATE_BUFFER|#FORMAT_MESSAGE_FROM_SYSTEM,0,GetLastError_(),0,@Buffer,0,0)
  result$= PeekS(Buffer)
  LocalFree_(Buffer)
  ProcedureReturn result$
EndProcedure



Procedure Copy(List files.files())
  Define lenFrom,lenDest
  Define *memFrom.character,*posFrom.character
  Define *memDest.character,*posDest.character
  
  lenFrom=2
  lenDest=2
  ForEach files()
    lenFrom+ ((Len(files()\from)+1)*SizeOf(character))
    lenDest+ ((Len(files()\dest)+1)*SizeOf(character))
    Debug files()\from +" "+Len(files()\from)
  Next
  *memFrom=AllocateMemory(lenFrom)
  *memDest=AllocateMemory(lenDest)
  
  *posFrom=*memFrom
  *posDest=*memDest
  ForEach files()
    CopyMemoryString(files()\from,@*posFrom)
    CopyMemoryString(files()\dest,@*posDest)
    *posFrom+SizeOf(character)
    *posDest+SizeOf(character)
  Next
  
  
  Define k.SHFILEOPSTRUCT
  k\wFunc=#FO_COPY
  k\pFrom=*memFrom
  k\pto=*memDest
  k\fFlags=#FOF_MULTIDESTFILES | #FOF_NOCONFIRMATION | #FOF_NOCONFIRMMKDIR | #FOF_ALLOWUNDO
  If SHFileOperation_(k)=#Null
    Debug "Copy Ok"
  EndIf
  Debug GetLastErrorMsg()
    
  FreeMemory(*MemDest)
  FreeMemory(*memFrom)
  
EndProcedure

das Ding ruft den Windows-Kopierdialog auf. Das kopieren wird auch in die Windows-Undo-Funktion eingebunden. Es wird immer auf das ende der Kopieraktion gewartet.
Eventuell eine Alternative, anstatt immer das Rad neu so erfinden.

[Nino]

Eventuell eine Alternative, anstatt immer das Rad neu so erfinden.

Vielen Dank für den Code, das ist sehr freundlich.

Aber ein wichtiger Zweck meines Programms ist, dass ich Erfahrungen mit Threads in PB sammle, von daher muss ich es schon selbst schreiben.
Dank Eurer Hilfe habe ich dafür jetzt auch eine Grundlage.

[X0r]

Ist zwar ein wenig her, aber: ich finde den aufgeführten Code-Gerüst nicht so optimal. Für eine GUI Anwendung solltest du die Implementierung einer richtigen Schichtenarchitektur (z.B. MVC oder besser: MVP) vornehmen. Das bringt neben der Wartungsfreundlichkeit zahlreiche andere Vorteile.
Schichtenarchitekturen wie MVC/MVP werden zwar oft in Verbindung mit dem objektorientierten Paradigma umgesetzt, allerdings sind Schichtenarchitekturen meines Wissens nach grundsätzlich unabhängig vom verwendeten Paradigma. Seit der Einführung von Modulen sollte eine ordentliche Implementierung in PureBasic auch möglich sein.