wlitke / translator

Translator application
2 stars 0 forks source link

11 sound devices #18

Closed eppstephan closed 1 year ago

eppstephan commented 1 year ago

@wlitke Eventuell könnte man die User Eingabe noch etwas robuster gegen falsche Eingaben machen. Ich persönlich finde die geschmissenen Exceptions schon recht aussagekräftig, aber ich bin zu sehr im Code. Dennoch, zum Testen reicht die Version vielleicht schon aus. Vielleicht könnte man noch ein Refactoring durchführen, aber noch hält sich die Klasse Program von ihrer Größe her in Grenzen, denke ich (Stichwort: God-Klasse vermeiden). Ansonsten habe ich eine Klasse AudioDevice (ohne extra Datei) erstellt, was nicht unbedingt notwendig ist (aber zur Ausgabe des Namens z.B. könnte es wieder interessant werden).

wlitke commented 1 year ago

@eppstephan Ich habe den aktuellen Code-Stand getestet und er funktioniert ausgesprochen gut. Kompliment bis hierhin! Wenn man den "Stereo Mix" Eingang aktiviert (um jegliche Lautsprecher-Ausgabe als Eingabe zu nutzen) sind die Ergebnisse eher mäßig. Ich habe auf die Schnelle nicht herausfinden können, woran dies liegt. Selbst wenn die Ton-Datei sehr gut zu verstehen ist (Hier ein Beispiel von mir: https://www.dropbox.com/s/ls4krzkd2teqagf/Sprachtest.mp3?dl=0), kommt das Programm mit Pausen und darauf folgenden Sätzen nur mäßig zurecht.

Bezüglich der Exceptions würde ich auf jeden Fall lieber eine konkrete Meldung, als eine Exception Meldung sehen. Letzte lässt den Eindruck aufkommen, dass sie nicht behandelt wurde bzw. ein unbehandelter Programmzustand eingetreten ist. Wenn zum Beispiel ein ungültiger/falscher SubscriptionKey eingegeben wurde, erhält man folgende Exception: Recognition canceled. Reason: Error; ErrorDetails: WebSocket upgrade failed: Authentication error (401). Please check subscription information and region name. SessionId: 3adda19507a24dc995e7c5a42dca504a Hier wäre eine konkrete Fehler-Meldung mit einer Handlungsempfehlung besser, z. B. "The configured SubscriptionKey is invalid. Check the App.config file for the right value."

Im Übrigen sollte die Eingabe ebenfalls so robust sein, dass sie nur numerische und existierende Werte entgegennehmen kann. Im Falle einer falschen Ausgabe sollte folgende Meldung ausgegeben werden und die Eingabe erneut vorgenommen werden, z. B. "The entered value is invalid. Please enter an existing number."

Abschließend noch ein Hinweis zum Refactoring: Hier sollten wir auf jeden Fall dran, sobald die Anderen die bestehende Funktionalität bestätigt haben. Andernfalls nehmen wir Veränderungen/Verbesserungen vor, die anschließend möglicherweise verworfen werden müssen.

eppstephan commented 1 year ago

@wlitke Könntest du ein Review durchführen? Ich habe einige Änderungen vorgenommen hinsichtlich der Robustheit bei der Eingabe einer Nummer zur Auswahl des Devices und der Fehlermeldungen im Falle einer fehlerhaften Konfiguration (Subscription Key, Region, ...).

Vom Gefühl her ist mein Eindruck, dass man das Programm mal in einem realen Umfeld testen lassen könnte. Wie denkst du?

Eventuell könnte man noch den Link zu den unterstützen Sprachen von Microsoft z.B. in der App Config als Kommentar bereitstellen.

wlitke commented 1 year ago

@eppstephan Die Anpassungen sehen soweit gut aus. Ja, ich denke, du hast Recht; aus Funktionsperspektive ist die Anwendung tatsächlich für einen ersten Test reif. Bevor wir die Anwendung jedoch weitergeben, sollten wir noch folgende (nicht unwichtige) Anpassungen durchführen:

eppstephan commented 1 year ago

@wlitke Sollte nur die Assembly Version oder auch die Assembly File Version angepasst werden?

wlitke commented 1 year ago

@eppstephan Eigentlich dürfte die AssemblyFileVersion genügen. In erster Linie soll über den Datei Explorer die Version eingesehen und von anderen Versionen abgegrenzt werden können. Die AssemblyVersion wird eher intern verwendet und ist weniger relevant.

PS: Hier ist eine Erklärung der beiden Attribute und ihre übliche Verwendung:

https://learn.microsoft.com/de-de/troubleshoot/developer/visualstudio/general/assembly-version-assembly-file-version

eppstephan commented 1 year ago

@wlitke Könntest du ein Review durchführen? Die README.md habe ich vom Aufbau her so gelassen. Insbesondere aber das Bearbeiten der App.config habe ich erwähnt (mit Verweis auf die Liste aller zu unterstützenden Sprachen). Eventuell könnte man in der README.md das Programm noch genauer beschreiben z.B. hinsichtlich der Auswahl der Audio Ein- und Ausgänge. Ich habe das aber erst mal gelassen.