C# Feedback zum Projekt - tone: audio tagger in C#

sandreas

Lieutenant
Registriert
Juli 2012
Beiträge
562
Hallo zusammen,

wünsche mir Feedback zum meinem neuesten Projekt:

https://github.com/sandreas/tone

Ist ein Cross-Platform Kommandozeilen-Programm zum Anzeigen und Bearbeiten von Tags in Audio-Dateien geschrieben in C# - langfristig gedacht als Ergänzung bzw. sogar Ersatz für m4b-tool. Besonderes Augenmerk dabei auf:
  • User-Interface / Usability (Bugs und Unklarheiten)
  • Doku
  • Fehlende Features
  • Gerne auch: Code-Qualität

Ich habs bisher außer auf github noch nirgendwo sonst gepostet, das Projekt ist noch relativ neu. Aktuell sind noch folgende Punkte auf meiner Todo-Liste:

  • Usability Beispiel GIF mit terminalizer erstellen
  • split Funktion zum Aufteilen von Hörbüchern mit Kapiteln in einzelne Dateien
  • merge Funktion um mehrteilige Hörbücher zusammenzufassen
  • ein offizielles docker-image
  • Unit-Tests schreiben um bestehende Funktionen abzudecken

Das soll keine Werbung sein, hoffe, ich verstoße damit nicht gegen irgendwelche Foren-Regeln, ansonsten gerne zu machen und mich kurz darüber informieren.

Vielen Dank
 
Hm? Hier meldet sich irgendwie keiner. Na vielleicht ist CB einfach nicht (mehr) der richtige Ort für sowas.... 🤔

So ein paar Ideen, die sich mir beim Drüberschauen ergeben haben:
- Etwas viele Optionen. Und wenn ich das richtig verstanden habe, dann werden an manchen Stellen auch dieselbe Option mehrfach angegeben, um "mehrere" Einträge für eine Option zu bekommen.
=> Das macht es natürlich für alle Beteiligten, Dich wie den Anwender auch, zu einem ziemlichen Tohuwabohu. Daher: Optionen einkürzen und, wo es praktikabel ist, statt dieselbe Option mehrfach angeben zu müssen, eine Option als Liste auswertbar zu machen. ZB --option=A,B,C oder sowas.
Aber auch ein Templateformat wäre eine Idee. Das hast Du ja stellenweise schon.
Weitergehend wäre auch eine Form einer Templatedatei möglich, die man einmal erstellt und die man dann zB per --config <name> auswählen kann.

- Mich persönlich ödet das mit dem "var" in .Net schon seit Ewigkeiten an... das kannst Du natürlich halten wie Du möchtest, aber so als Vorschlag: net6+ erlaubt es auch, die Typinferenz "rumzudrehen". Also statt var x=new Fileinfo() halt FileInfo x = new() .

- Du hast in der global.json angegeben, daß rollforward auf "irgendwas" möglich sein soll. Wäre ich vorsichtig mit. Bist Du sicher, daß das auf net7, net8, net irgendwas immer noch laufen wird?

-- Dependency injection. Versuch das loszuwerden, wenn irgend möglich. MS hat schon angedroht, das stopfen zu wollen.

-- OO macht es natürlich naturgemäß immer etwas schwieriger, "schnell" einen tiefen Einblick zu bekommen, aber mir fällt beim Lesen auf, daß Du evtl. von yields gebrauch machen könntest (wenn Du nicht bereits Dich explizit dagegen entschieden hattest). Aber das nur so als Überlegung, wo man hingucken könnte; vor allem wenn Du feststellst daß es mancherorts vielleicht unnötig hängt.

Gibt sicher noch einiges mehr, aber das ist erstmal das was mir aufgefallen ist.

Alles in allem wirkt der Code jedenfalls strukturiert (daß es inzwischen file-scoped namespaces gibt und ich die auf den Tod nicht abkann ist ja nicht Dein Vergehen :D). Definitiv besser als eine Menge Code wo ich mich mit auseinandersetzen "durfte".

Ausnahmen hab ich allerdings keine gesehen. Will natürlich nix heißen (hab nicht in jede Datei geschaut), daher: definierst Du irgendwo welche und nutzt sie auch?

Dasselbe für Events. Beides ist natürlich kein Muß, aber ich unterstelle mal, daß Dir ein Exception- und ein Eventmodell bei der Implementierung durchaus entgegenkommen können, wenn nämlich eine Aktion zB "bin fertig" signalisieren kann oder wenn du feststellst, Mist, der Vorgang hat zwar funktioniert, aber nicht wie er sollte, weil XYZ ist passiert. Manchmal(tm) kann man sowas ja abfangen ("die Datei die du gesagt hast gibt es gar nicht => gib neu ein" oder "hey in /tmp kann ich nicht schreiben, aber vielleicht nach $USER/tmp?"). Manchmal aber auch nicht ("ja das war ja alles super schick, aber Dateisystem voll, kann ich ja nicht riechen" oder "ich weiß nicht wie du das geschafft hast aber deine Laufzeitumgebung ist so kaputt daß ich damit nix mehr machen kann").


Finally, dotnet build, osx12, net7 7.0.100, net6 6.0.300, code unverändert:

Code:
Microsoft (R) Build Engine version 17.3.0-preview-22226-04+f15ed2652 for .NET
Copyright (C) Microsoft Corporation. All rights reserved.

  Determining projects to restore...
tone/tone.Tests/tone.Tests.csproj : warning NU1603: tone.Tests depends on Sandreas.Files (>= 0.0.5) but Sandreas.Files 0.0.5 was not found. An approximate best match of Sandreas.Files 1.0.0 was resolved. [tone/tone.sln]
tone/tone/tone.csproj : warning NU1603: tone depends on Sandreas.Files (>= 0.0.5) but Sandreas.Files 0.0.5 was not found. An approximate best match of Sandreas.Files 1.0.0 was resolved. [tone/tone.sln]
  All projects are up-to-date for restore.
/usr/local/share/dotnet/sdk/7.0.100-preview.4.22252.9/Sdks/Microsoft.NET.Sdk/targets/Microsoft.NET.RuntimeIdentifierInference.targets(216,5): message NETSDK1057: You are using a preview version of .NET. See: https://aka.ms/dotnet-support-policy [tone/tone.Tests/tone.Tests.csproj]
tone/tone.Tests/tone.Tests.csproj : warning NU1603: tone.Tests depends on Sandreas.Files (>= 0.0.5) but Sandreas.Files 0.0.5 was not found. An approximate best match of Sandreas.Files 1.0.0 was resolved.
/usr/local/share/dotnet/sdk/7.0.100-preview.4.22252.9/Sdks/Microsoft.NET.Sdk/targets/Microsoft.NET.RuntimeIdentifierInference.targets(216,5): message NETSDK1057: You are using a preview version of .NET. See: https://aka.ms/dotnet-support-policy [tone/tone/tone.csproj]
tone/tone/tone.csproj : warning NU1603: tone depends on Sandreas.Files (>= 0.0.5) but Sandreas.Files 0.0.5 was not found. An approximate best match of Sandreas.Files 1.0.0 was resolved.
tone/tone/Services/DirectoryLoaderService.cs(40,50): error CS1061: 'FileWalker' does not contain a definition for 'FileSystem' and no accessible extension method 'FileSystem' accepting a first argument of type 'FileWalker' could be found (are you missing a using directive or an assembly reference?) [tone/tone/tone.csproj]
tone/tone/Services/DirectoryLoaderService.cs(75,35): error CS1503: Argument 1: cannot convert from 'string' to 'System.IO.Abstractions.IFileInfo' [tone/tone/tone.csproj]
tone/tone/Services/DirectoryLoaderService.cs(77,41): error CS1061: 'FileWalker' does not contain a definition for 'FileSystem' and no accessible extension method 'FileSystem' accepting a first argument of type 'FileWalker' could be found (are you missing a using directive or an assembly reference?) [tone/tone/tone.csproj]
tone/tone/DependencyInjection/TypeRegistrar.cs(23,9): warning IL2067: 'implementationType' argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicConstructors' in call to 'Microsoft.Extensions.DependencyInjection.ServiceCollectionServiceExtensions.AddSingleton(IServiceCollection, Type, Type)'. The parameter 'implementation' of method 'tone.DependencyInjection.TypeRegistrar.Register(Type, Type)' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to. [tone/tone/tone.csproj]
tone/tone/Common/Extensions/Object/ObjectExtensions.cs(41,16): warning IL2090: 'this' argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicProperties' in call to 'System.Type.GetProperties()'. The generic parameter 'T' of 'tone.Common.Extensions.Object.ObjectExtensions.GetProperties<T>(T)' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to. [tone/tone/tone.csproj]

Build FAILED.

tone/tone.Tests/tone.Tests.csproj : warning NU1603: tone.Tests depends on Sandreas.Files (>= 0.0.5) but Sandreas.Files 0.0.5 was not found. An approximate best match of Sandreas.Files 1.0.0 was resolved. [tone/tone.sln]
tone/tone/tone.csproj : warning NU1603: tone depends on Sandreas.Files (>= 0.0.5) but Sandreas.Files 0.0.5 was not found. An approximate best match of Sandreas.Files 1.0.0 was resolved. [tone/tone.sln]
tone/tone.Tests/tone.Tests.csproj : warning NU1603: tone.Tests depends on Sandreas.Files (>= 0.0.5) but Sandreas.Files 0.0.5 was not found. An approximate best match of Sandreas.Files 1.0.0 was resolved.
tone/tone/tone.csproj : warning NU1603: tone depends on Sandreas.Files (>= 0.0.5) but Sandreas.Files 0.0.5 was not found. An approximate best match of Sandreas.Files 1.0.0 was resolved.
tone/tone/DependencyInjection/TypeRegistrar.cs(23,9): warning IL2067: 'implementationType' argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicConstructors' in call to 'Microsoft.Extensions.DependencyInjection.ServiceCollectionServiceExtensions.AddSingleton(IServiceCollection, Type, Type)'. The parameter 'implementation' of method 'tone.DependencyInjection.TypeRegistrar.Register(Type, Type)' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to. [tone/tone/tone.csproj]
tone/tone/Common/Extensions/Object/ObjectExtensions.cs(41,16): warning IL2090: 'this' argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicProperties' in call to 'System.Type.GetProperties()'. The generic parameter 'T' of 'tone.Common.Extensions.Object.ObjectExtensions.GetProperties<T>(T)' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to. [tone/tone/tone.csproj]
tone/tone/Services/DirectoryLoaderService.cs(40,50): error CS1061: 'FileWalker' does not contain a definition for 'FileSystem' and no accessible extension method 'FileSystem' accepting a first argument of type 'FileWalker' could be found (are you missing a using directive or an assembly reference?) [tone/tone/tone.csproj]
tone/tone/Services/DirectoryLoaderService.cs(75,35): error CS1503: Argument 1: cannot convert from 'string' to 'System.IO.Abstractions.IFileInfo' [tone/tone/tone.csproj]
tone/tone/Services/DirectoryLoaderService.cs(77,41): error CS1061: 'FileWalker' does not contain a definition for 'FileSystem' and no accessible extension method 'FileSystem' accepting a first argument of type 'FileWalker' could be found (are you missing a using directive or an assembly reference?) [tone/tone/tone.csproj]
    6 Warning(s)
    3 Error(s)

Time Elapsed 00:00:00.73
Ich geh mal davon aus, daß das zunächst an der unbeschränkten global.json liegt und daß ein passender Tools-Eintrag dort schon helfen würde, hab es aber auch nicht weiter verfolgt, sorry. :daumen:
 
  • Gefällt mir
Reaktionen: andy_m4 und sandreas
@RalphS
Cool, erstmal vielen Dank für den ausführlichen und wirklich hilfreichen Beitrag.

RalphS schrieb:
Hm? Hier meldet sich irgendwie keiner. Na vielleicht ist CB einfach nicht (mehr) der richtige Ort für sowas.... 🤔
Naja, ein ganzes Projekt zu reviewen braucht ja auch Zeit. Daher bin ich dir umso dankbarer, dass du dir die Zeit genommen hast :-)

RalphS schrieb:
So ein paar Ideen, die sich mir beim Drüberschauen ergeben haben:
- Etwas viele Optionen. Und wenn ich das richtig verstanden habe, dann werden an manchen Stellen auch dieselbe Option mehrfach angegeben, um "mehrere" Einträge für eine Option zu bekommen.
=> Das macht es natürlich für alle Beteiligten, Dich wie den Anwender auch, zu einem ziemlichen Tohuwabohu. Daher: Optionen einkürzen und, wo es praktikabel ist, statt dieselbe Option mehrfach angeben zu müssen, eine Option als Liste auswertbar zu machen. ZB --option=A,B,C oder sowas.
Aber auch ein Templateformat wäre eine Idee. Das hast Du ja stellenweise schon.
Weitergehend wäre auch eine Form einer Templatedatei möglich, die man einmal erstellt und die man dann zB per --config <name> auswählen kann.
Hervorragende Anmerkungen. Da kann ich weitgehend nur zustimmen - Hintergründe sind:
  • Es gibt einfach viele Tags... ob ich die tatsächlich auch alle als Option brauche ist tatsächlich fragwürdig. Ein Dateiimport wäre eventuell besser
  • Mehrfachoptionen waren mit der CommandLine-Library (spectre.console, die ist hervorragend) einfach umsetzbar, ich habe aber auch schon ein Ticket mit der selben Idee aufgemacht (https://github.com/spectreconsole/spectre.console/issues/826)
  • Ich hatte auch an Environment-Variablen oder eine Konfigurationsdatei für einige Dinge gedacht, das ist wirklich ein guter Punkt
RalphS schrieb:
- Mich persönlich ödet das mit dem "var" in .Net schon seit Ewigkeiten an... das kannst Du natürlich halten wie Du möchtest, aber so als Vorschlag: net6+ erlaubt es auch, die Typinferenz "rumzudrehen". Also statt var x=new Fileinfo() halt FileInfo x = new() .
Witzig, dass du das sagst... die meisten "Old-School" C# Entwickler sehen das so(man beachte: ich meine nicht altmodisch, sondern mit langjähriger Erfahrung). Ich persönlich mag var gern. Ich mach C# aber auch erst seit 2 Jahren.

RalphS schrieb:
- Du hast in der global.json angegeben, daß rollforward auf "irgendwas" möglich sein soll. Wäre ich vorsichtig mit. Bist Du sicher, daß das auf net7, net8, net irgendwas immer noch laufen wird?
Guter Hinweis, damit hab ich mich gar nicht befasst. Das wurde entweder von meiner IDE oder von dotnet automatisch erstellt.

RalphS schrieb:
-- Dependency injection. Versuch das loszuwerden, wenn irgend möglich. MS hat schon angedroht, das stopfen zu wollen.
WAAAS? DI / IoC kommt weg? Hast du dazu ne Quelle? In allen Tutorials wurde das empfohlen oder sogar vorausgesetzt... Das würde mich aber sehr wundern.

RalphS schrieb:
-- OO macht es natürlich naturgemäß immer etwas schwieriger, "schnell" einen tiefen Einblick zu bekommen, aber mir fällt beim Lesen auf, daß Du evtl. von yields gebrauch machen könntest (wenn Du nicht bereits Dich explizit dagegen entschieden hattest). Aber das nur so als Überlegung, wo man hingucken könnte; vor allem wenn Du feststellst daß es mancherorts vielleicht unnötig hängt.
Ja, das stimmt - ist notiert. Performance/Speicher-Optimierung hab ich noch irgendwo auf meiner Todo-Liste. IEnumerable habe ich inzwischen ganz gut im Griff glaube ich. In meiner Filewalker-Library, die auch in tone zum Einsatz kommt (https://github.com/sandreas/DotnetLibFiles/blob/main/Sandreas/Files/FileWalker.cs) habe ich das tatsächlich schon genutzt.

RalphS schrieb:
daß es inzwischen file-scoped namespaces gibt und ich die auf den Tod nicht abkann ist ja nicht Dein Vergehen :D
Ich sag ja... Old-School im positivsten Sinne :-) Ich mags...

RalphS schrieb:
Ausnahmen hab ich allerdings keine gesehen. Will natürlich nix heißen (hab nicht in jede Datei geschaut), daher: definierst Du irgendwo welche und nutzt sie auch?
Ich bin inzwischen kein großer Fan mehr von Exceptions (früher war ichs), daher verwende ich in dem Projekt bewusst die Library OperationResult. Das ist aber auch Geschmackssache. Siehe dazu auch meinen Beitrag hier und hier, sofern du die Zeit investieren möchtest :-)

RalphS schrieb:
Dasselbe für Events. Beides ist natürlich kein Muß, aber ich unterstelle mal, daß Dir ein Exception- und ein Eventmodell bei der Implementierung durchaus entgegenkommen können, wenn nämlich eine Aktion zB "bin fertig" signalisieren kann oder wenn du feststellst, Mist, der Vorgang hat zwar funktioniert, aber nicht wie er sollte, weil XYZ ist passiert. Manchmal(tm) kann man sowas ja abfangen ("die Datei die du gesagt hast gibt es gar nicht => gib neu ein" oder "hey in /tmp kann ich nicht schreiben, aber vielleicht nach $USER/tmp?"). Manchmal aber auch nicht ("ja das war ja alles super schick, aber Dateisystem voll, kann ich ja nicht riechen" oder "ich weiß nicht wie du das geschafft hast aber deine Laufzeitumgebung ist so kaputt daß ich damit nix mehr machen kann").
Ja über Events habe ich nachgedacht. Mit async/await und Tasks bin ich bisher ganz gut drum rum gekommen, aber hier habe ich noch Defizite, an denen ich arbeiten muss. Vielleicht kommts noch rein, ich habe sehr gutes über MediatR gelesen (hoffenltich war das auf die schnelle die richtige Lib). Schaue ich mir aber definitiv noch mal an, wenns ans Splitten und Mergen von Dateien geht :-)

RalphS schrieb:
Finally, dotnet build, osx12, net7 7.0.100, net6 6.0.300, code unverändert:
Danke für den Hinweis, hier habe ich noch ein Problem mit der externen Filewalker-Library, die ich erstellt habe. Ich wollte die nicht bei nuget publishen, weil es mich nervt, dass man dort keine Versionen entfernen kann und die Lib war noch ziemlicher Bastelkram. Die github package registry ist aber glaube ich nicht öffentlich zugreifbar. Ich hab es zwar geschafft, die Library und tone automatisch beim erstellen eines git tags via github Action bauen und publishen zu lassen (siehe github actions für tone und Filewalker), aber irgendwas ist da noch nicht ganz richtig.


Alles in allem sehr berechtigte und vor allem hilfreiche Kritik.
Vielen vielen Dank dafür, ich werde mich bemühen, alles zu berücksichtigen und umzusetzen. Ich hoffe, meine Erklärungen werden deiner Mühe gerecht :-)
 
:daumen: nur nicht zu überschwenglich, mein kopf ist schon so zu aufgeblasen :D

Hab nochmal wegen DI geschaut und es sieht so aus, als hätt ich da irgendwo was zuviel gelesen oder durcheinandergewürfelt oder keine Ahnung, jedenfalls find ich keine Referenz (mehr?) dazu. Sorry. 😊

Jau, vielleicht bin ich zu oldschool, das merk ich durchaus ab und zu auch selber. Aber manchmal ist es (imo) auch hilfreich. "var xyz" sagt mir halt erstmal gar nix, wenn ich zB nen LINQ Expression hab und ein var dazu, dann ist das irgendwas, aber nicht unbedingt das was ich "wollte" oder "brauchte". Select vs SelectMany? Meh.

Type x = ... fliegt halt sofort um die Ohren, wenn es nicht paßt.

Exceptions, eh, da zu diskutieren ist glaub ich müßig. Für mich gehört das zu OO einfach dazu, aber wie Du richtig erkannt hast bin ich oldschool und muß aufpassen daß ich mich nicht von der Realität überfahren lasse. Demgegenüber haben mE Statuscodes in OO nichts zu suchen. Da will ich zB try/catch/finally sagen und nicht if/then plus "irgendwas" -- nach "Neusprech" ist das halt auch ein Pattern, try/catch/finally sagt sofort "aha Ausnahmebehandlung" und switch(Statuscode) ... tut das halt nicht.

Aber wie gesagt, YMMV.
 
  • Gefällt mir
Reaktionen: sandreas
RalphS schrieb:
Hab nochmal wegen DI geschaut und es sieht so aus, als hätt ich da irgendwo was zuviel gelesen oder durcheinandergewürfelt oder keine Ahnung, jedenfalls find ich keine Referenz (mehr?) dazu. Sorry. 😊
Gut, da bin ich froh, das hätt mich jetzt echt aus den Socken gehauen.

RalphS schrieb:
Jau, vielleicht bin ich zu oldschool, das merk ich durchaus ab und zu auch selber. Aber manchmal ist es (imo) auch hilfreich.
Wie man es auch nennen mag... wenn man in jeden Hype-Train einsteigt, kriegt man auch nix fertig, deshalb kenn ich das Gefühl - programmiere nun auch schon ne weile. Ich bin inzwischen moderneren Ansätzen aber aufgeschlossen, insbesondere, da ich "neue Konzepte lernen" für das wichtigste bei der Weiterentwicklung halte. Tools, Programmiersprachen und Frameworks kommen und gehen, Konzepte bleiben. Ich persönlich mag die Flexibilität von var, man muss oft weniger Code schreiben und bei größeren Refactorings weniger Stellen ändern. Aber jedem das seine.

RalphS schrieb:
Exceptions, eh, da zu diskutieren ist glaub ich müßig.
Vermutlich :-)

RalphS schrieb:
Demgegenüber haben mE Statuscodes in OO nichts zu suchen. Da will ich zB try/catch/finally sagen und nicht if/then plus "irgendwas" -- nach "Neusprech" ist das halt auch ein Pattern, try/catch/finally sagt sofort "aha Ausnahmebehandlung" und switch(Statuscode) ... tut das halt nicht.
OperationResult ist nicht mit Statuscodes realisiert, sondern mit so genannten Wrapper-Types. Du definierst bei Methoden via Generics einen SuccessType und einen ErrorType. Beide implementieren den implicit operator bool, um zu prüfen, ob es erfolgreich war. Ein ErrorResult ist false, ein SuccessResult true. Sieht dann so aus:


C#:
public static Result<IEnumerable<Grok>, string> machWas() {

// ...

}

public static int Main(string[] args) {

   var result = machWas();

   if(!result) {

       // error
       Console.WriteLine(result.Error);
       return 1;

   }

   // sucess
   foreach(var grok in result.Value) {
   // ...
   }

   return 0;
}

SucessType ist IEnumerable<Grok> und der ErrorType nur ein string. Probiers mal aus, ist echt nicht viel Arbeit.

Konkreteres Beispiel: https://github.com/sandreas/tone/bl...ca0fe/tone/Services/GrokPatternService.cs#L17
 
Bin erstmal nur kurz mit meiner Architekturbrille drübergeflogen und dabei sind mir 2 Punkte aufgefallen:

  • Libs werden nicht enkoppelt
  • Die Ordnerstruktur gleicht gefühlt jedem anderen Projekt und „erzählt“ mir nichts

Der erste Punkt ist natürlich stark vom Anspruch der Applikation abhängig. Wenn das nur was kleines und überschaubares bleiben soll ist das natürlich weniger tragisch, da du etwaige Packages relativ schnell wieder ausgetauscht bekommst
Wenn du aber planst, dass das mit der Zeit ein großes Projekt werden soll, das auch in die Zukunft skalieren soll, dann rate ich stark zur Entkopplung externer Abhängigkeiten.
Im Laufe meiner Jahre als Entwickler habe ich nicht wenige Projekte begleitet, die sich genau deswegen zu wirtschaftlichen Totalschäden entwickelt haben.

Der 2. Punkt ist etwas, das ich auch sehr oft sehe: Jedes Projekt sieht gleich aus.
Es gibt Services, Commands, Helper, Utlity usw.
Ich empfehle immer eine „Screaming Architecture“ zu verfolgen. Das hat den Vorteil, dass man durch das bloße draufschauen auf die Ordnerstruktur erahnen kann, was deine Software überhaupt macht. Das hilft mir persönlich extrem beim Zurechtfinden & Einarbeiten.
 
  • Gefällt mir
Reaktionen: sandreas
@VD90 Danke für dein Feedback. Bin grad auf m Weg ins Bett, aber ich nehm mir die Tage noch mal Zeit, deine Anmerkungen entsprechend zu würdigen.

Sind auf jeden Fall valide Punkte und ich bin gespannt auf ein Beispiel zur Screaming Architektur. Danke sehr.
 
Naja, halt keine generische Struktur. Ich bin auch über das ./var geflogen - das hat da nicht wirklich was verloren, aber ich dachte mir, meh. 🤷‍♀️
Also nicht so:
classes/class1.cs
classes/class2.cs
ressources/image.png
forms/form1.cs

Sondern eher:
cmdlets/GetFileSystemInfo.cs
xaml/Mainwindow.xaml (+.cs)
api/IBrowserInterface.cs

und so weiter.
Zumindest ist das meine Interpretation einer "screaming architecture" 😅
 
  • Gefällt mir
Reaktionen: sandreas
So, dann kann ich jetzt noch mal Fragen zu deinem Feedback stellen:
VD90 schrieb:
Der erste Punkt ist natürlich stark vom Anspruch der Applikation abhängig. Wenn das nur was kleines und überschaubares bleiben soll ist das natürlich weniger tragisch, da du etwaige Packages relativ schnell wieder ausgetauscht bekommst
Aktuell ist es noch ein sehr kleines command line tool. Da meine Zeit in Bälde sehr begrenzt sein wird, gehe ich davon aus, dass es auch nicht so bald darüber hinaus geht. Allerdings habe ich schon drüber nachgedacht, eine Plugin-Schnittstelle zu implementieren, dass man selbst Tagger-Plugins entwickeln kann. Von daher wäre es sicher schön, diese Architektur vorbereiten zu können.
Geplant ist konkret, m4b-tool abzulösen (langfristig). Das ganze dann mit echtem Multithreading. Ein OS-abhängiger Downloader für die Abhängikeiten (ffmpeg, fdkaac) wäre auch nice to have, damit die Anwendung funktioniert, ohne das man großen Einrichtungsaufwand hat.

VD90 schrieb:
Libs werden nicht enkoppelt

Kannst du mir das an einem konkreten Beispiel noch mal zeigen? Am Beispiel atldotnet habe ich versucht, ein eigenes IMetadata Interface zu erstellen und den Track nicht direkt zu verwenden, sondern im MetadataTrack zu wrappen. Bei grok habe ich einen Service geschrieben. Du hast aber recht, das ist recht eng an die libs gekoppelt. Ich bin tatsächlich ein großer Fan von YAGNI, allerdings werde ich noch mal schauen, ob eine Abstraktion in einigen Fällen Sinn macht.

VD90 schrieb:
Der 2. Punkt ist etwas, das ich auch sehr oft sehe: Jedes Projekt sieht gleich aus.
Es gibt Services, Commands, Helper, Utlity usw.
Ich empfehle immer eine „Screaming Architecture“ zu verfolgen.
Das finde ich sehr gut. Hier kann ich sicher viel restrukturieren und sozusagen in "Module" aufteilen. Das würde auch zu der oben genannten Plugin-Architektur.

Vielen Dank nochmal und ich bin gespannt, was noch aus dem Tool wird. Ihr seid herzlich eingeladen, Pull Requests zu stellen :p Die Mac M1 Version scheint schon mal nicht zu klappen. Mal schauen, was da schief läuft :-)
 
Generelles Lob für das Veröffentlichen und das sich zur Schau Stellen.

Kritik in beide Richtungen möchte ich mir daher nicht anmaßen.
 
  • Gefällt mir
Reaktionen: sandreas
Danke :-) Ich habe viele meiner besten und lehrreichsten Erkenntnisse durch das Veröffentlichen von Open Source Projekten gewonnen... daher mach ich das gern, um sinnvolles Feedback zu erhalten.

In diesem Sinne:
Ausführen kann man das dann so:
Bash:
docker run sandreas/tone:v0.0.5 dump --help 

# wahlweise einen alias erstellen
alias tone="docker run sandreas/tone:v0.0.5"
tone dump --help

Falls jemand sich bereit erklärt, das mal unter nem Raspi 3 oder 4 zu testen, würde ich mich freuen. Das x64 docker image funktioniert bei mir schon mal tadellos, aber damit gibt es auch selten Probleme.
 
  • Gefällt mir
Reaktionen: Boa-P und Wasserhuhn
nur mal nachgefragt, du hast da schönes desktop tool, und scheinst so hipster web developer schaizz wie docker einzubauen. warum?
 
Überleg dir mal noch unter welcher Lizenz das laufen soll, sonst darf das an sich gar keiner außer dir nutzen.
 
  • Gefällt mir
Reaktionen: sandreas
Oma Erna schrieb:
nur mal nachgefragt, du hast da schönes desktop tool, und scheinst so hipster web developer schaizz wie docker einzubauen. warum?
Docker ist nicht NUR hipster developer schaizz, sondern auch eine Möglichkeit, eine ganze Batterie von Abhängigkeiten sinnvoll zu verpacken und zusätzlich andere Software, die deine eigene nutzt mit deiner als Abhängigkeit zu versorgen. Außerdem ist Docker ja nicht exklusiv, sondern additiv, sprich: Man braucht es nicht, kann es aber verwenden. Beispiele:
  • tone wird auf lange Sicht ziemlich sicher ffmpeg brauchen, um Audio-Dateien zu konvertieren. ffmpeg lässt sich leicht installieren, allerdings gibt es einen non-free codec (libfdk-aac) der eine wesentlich besser Audioqualität liefert. Dieser darf aber aus Lizenzgründen nicht mit ausgeliefert werden. Docker bietet hier die Möglichkeit, ein selbst kompiliertes, schlankes, statisches ffmpeg zu bauen und es ohne das auf dem System installierte ffmpeg zu überschreiben zu integrieren
  • m4b-tool ist ein Tool für Hörbücher, es ist in PHP geschrieben und folglich nicht sehr mächtig - es sei denn, man packt Abhängigkeiten dazu. Kürzlich habe ich tone integriert.
morcego schrieb:
Überleg dir mal noch unter welcher Lizenz das laufen soll, sonst darf das an sich gar keiner außer dir nutzen.
Guter Punkt, ich werde wohl die Apache Lizenz wählen denke ich :-) Danke.
 
So, neue Version 0.0.6

Hauptsächliche Änderung ist, dass nun ein eigenes Metadata-Format entwickelt wurde: tone.json.

Dieses kann in einer JSON-Datei so ziemlich alles speichern (inkl. Kapiteln und Bildern als Base64), was es an Metadaten gibt. Für die dump funktion wurde zusätzlich der Parameter --query hinzugefügt, wo man einen JSONPath-Ausdruck verwenden kann, um einen bestimmten Wert aus dem tone.json herauszufiltern. Will man z.B. nur den Namen des Albums haben, ginge das so:

Bash:
tone dump my-file.mp3 --format=json --query="$.meta.album"

Warum ein eigenes Format? Weil die Entwickler von spectre.console nicht in der Lage zu sein scheinen, Kommandozeilen-Argumente beginnend mit doppelten Anführungszeichen richtig zu parsen und es nicht als ihren Fehler ansehen, sondern es auf Microsoft schieben :-/ Hier würde ich mir auch noch Feedback von einem NICHT Windows Benutzer wünschen, ob das wirklich Microsoft verbockt hat. Ich glaube nämlich nicht. Es ist auch geplant, Support für Pipes / Stdin noch hinzuzufügen:

Bash:
echo '{"meta": {"album": "My Album"}}' | tone tag --stdin="json" a-music-file.mp3

Daher musste ein Format her, wo ich nicht auf Kommandozeilen-Argumente angewiesen bin, sondern wo es über eine Datei geht.

Viel Spaß beim Testen.
 
Zuletzt bearbeitet:
Warum zum Teufel ugly stupid json? Was ist falsch an XML?
Du kannst damit eine externe Lib einsparen was in dem Fall gut ist.
 
Oma Erna schrieb:
Warum zum Teufel ugly stupid json? Was ist falsch an XML?
Erstmal danke für dein Feedback. An XML wäre nix falsch. Aber tone hat eine JavaScript API (man kann selbst Skript-Tagger schreiben), daher ist JSON das praktikablere Format, um den Austausch einfach zu gewährleisten.

Oma Erna schrieb:
Du kannst damit eine externe Lib einsparen was in dem Fall gut ist.
Das ist grundsätzlich richtig, da ich JSON aber an vielen anderen Stellen brauche bzw. brauchen werde (API consumer), wird sich das nicht so sehr auswirken - die Lib ist eh drin. Trotzdem ein valider Punkt.
 
nichts hat eine js api, du willst dich an js anbiedern. würde ich so nicht machen, ist aber aber natürlich deine entscheidung.
in c# braucht man json nicht, meine meinung.
 
Oma Erna schrieb:
Warum zum Teufel ugly stupid json? Was ist falsch an XML?
Du kannst damit eine externe Lib einsparen was in dem Fall gut ist.
Warum um Gottes Willen sollte man freiwillig XML statt JSON verwenden?

Welche externe Lib spart man in dem Fall?

Imho macht @sandreas vieles richtig - auch Docker ist ne gute Entscheidung!
 
Zuletzt bearbeitet:
  • Gefällt mir
Reaktionen: Physikbuddha, sandreas und Boa-P

Ähnliche Themen

Zurück
Oben