Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Integrate Code Review findings #12

Open
ChristianSauer opened this issue May 15, 2020 · 0 comments
Open

Integrate Code Review findings #12

ChristianSauer opened this issue May 15, 2020 · 0 comments

Comments

@ChristianSauer
Copy link
Collaborator

Eigene Oberfläche statt SBA nutzen? SBA ist für .NET Entwickler nicht zugänglich, da auf Java/Spring ausgelegt

Der Begriff "Beans" ist in .NET unüblich -> Wenn du eine eigene Oberfläche erstellst, sollte das in Services umbenannt werden

Parametrisierung über services.AddOptions wirkt unüblich - libraries haben per default keine appsettings.json. https://docs.microsoft.com/de-de/aspnet/core/fundamentals/configuration/?view=aspnetcore-3.1

services.AddSingleton(services); // is this even a good idea? <--- das frage ich mich auch ;) Wir hatten in die Autofac-Doku geschaut - da sind irgendwo schöne Code-Beispiele für Zugriff auf den DI-Container via Reflection vergraben (https://autofaccn.readthedocs.io/en/latest/register/registration.html)

Ist die manuelle Implementierung der preflight-requests (OPTIONS) nötig? Ich kenne Core nicht gut genug, aber das lässt sich doch sicher global regeln -> https://docs.microsoft.com/de-de/aspnet/core/security/cors?view=aspnetcore-3.1

Exceptionhandling -> Ausgabe bei Exceptions eingrenzen, da sonst möglicherweise sensible Systemdaten via HTTP an den Client zurückgegeben werden.

schau dir mal die Konventionen der ASP.NET Web-API an - insbesondere sollten REST-Endpunkte eigentlich(!) asynchron implementiert sein (also Task zurückgeben). Ist in meinen Augen aber nur sinnvoll wenn man async-all-the-way geht und das durch die gesamte Anwendung zieht https://docs.microsoft.com/de-de/aspnet/core/tutorials/first-web-api?view=aspnetcore-3.1&tabs=visual-studio

EnvironmentProvider - der Code (insbesondere die verschachtelten foreach-Schleifen) sieht so aus, als ob er mit LINQ (.Aggregate() ?) deutlich eleganter geschrieben werden könnte

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant