A todo list for new ASP.NET Core projects
biarity.gitlab.io
biarity.gitlab.io
In the old days, that meant you could toss out all silly drag/drop, faux-stateful garbage they put in to make it so you could build webapps like they were VB6. Thus, leaving behind a really good compact MVC framework for building webapps (long before they came out with something actually called MVC that wasn't as good at it). Now it means that I can read this list and not see a single technology I've touched at all in my 18 years of build stuff on this stack.
Personally, I tend to disagree with the author's approach of pushing routine stuff out to the realm of "magically happens behind the scenes", in favor of having common base classes for my page handlers that can be explicitly told to do those things with a little Initialize() method at the top of the handler. That way, when something goes wrong I get a breakpoint looking at my own code.
I like control, so the idea of voluntarily putting something in place that could magically shove its way in front of my code and whisk the user off to an error page just smells wrong.
But again, it's cool that the stack gives us the option to do so if it fits our style.
For example, last I looked, the new configuration manager has to be injected.
Injected!
To me, who's not a particular fan of injection, that's insane. Especially given that the previous iteration was a static class that most of would just wrap in a static class called something like "Constants" or something.
These are global, static, variables. They shouldn't be changing during the application's run-time It should be super simple to use and fire-and-forget. Having to pass it into every constructor and then manage it is crazy to me.
Yeah, you can wrap the whole thing yourself, but it's just been done because DI is trendy at the moment. In my opinion, for a statically typed language, DI is just a pretty nasty language hack that's only there to facilitate test methods. You shouldn't be designing things around it.
But yes I agree injecting configuration seems sub-optimal at best.
Also, remember there's a huge swathe of apps where tests are wholly inappropriate and designing everything around DI is annoying for all those cases.
Prototypes, one time apps, internal tasks, the significant proportion of programmers who think testing's a waste of time and/or money, etc. or that only small, key parts should be tested.
The reason why you want to have it injected as IOption<> is cause you might want to change configuration on the fly. If you do it right, there is no need for the app to restart if you change the appsettings.json at all. Also it makes things much more flexible and loosely coupled, for example third party middlewares can bring their own Settings and you can define which part of the whole appsettings.json is meant to be for that Middleware.
There is also no need to pass it to the constructor and manage it yourself via a field inside your class, at least in your controllers. You can just use the [FromServices] attribute before an action argument to inject it into just that method (and that works for properties deeper down the "callstack" aswell).
I can see a case for mutable config, I use it, but the majority of config is not app time mutable and you shouldn't be designing a system for an edge case. Have a mutable config.
Experienced devs use the db for that by the way as mutable config generally needs an in app interface.
But especially for config, it's that if I'm making a tiny app, or just bashing something out, or prototyping, I don't want to spend a bunch of time setting up and configuring something as fundamental as config.
At the moment you add the System.Configuration library as soon as you start a project and out of the box you can just do:
var key = ConfigurationManager.AppSettings["SendGridAPIKey"]; //will be a string
With the newer features of C# you can put that on a class as simply as: public static class GlobalConfig
{
public static string SendGridAPIKey => ConfigurationManager.AppSettings["SendGridAPIKey"];
public static bool EmailsEnabled => bool.parse(ConfigurationManager.AppSettings["EmailsEnabled"]);
//etc.
}
Then access that anywhere in your code with the below. At worst you might have to add a namespace, which is literally ctrl+. then press space, if it happens to be in a different namespace: GlobalConfig.EmailEnabled
What I wanted from a new config manager is for it to be even simpler, to handle empty strings, etc. automatically. I don't see anything wrong with accessing those sort of config setting with the below. It's config, it's static, it's supposed to be globally accessed. It's a special sort of const that you're allowed to change without re-compiling. public static decimal MinimumOrderValue => Config.GetDecimalOrDefault("MinimumOrderValue");
(The OrDefaultValue there would mean 0 by .Net conventions for non-C#ers, if the string was null)Instead virtually every business class you make has to accept the config object, store it on a local variable and then access it through a local variable. Or you can spend a bunch of time wrapping the god-awful thing in your own static class or singleton (ugh) or whatever.
With the new way you have to do a bunch of extra work, write extra code. And what have I gained? Sod all. I can already change the config for unit tests by changing the config file in the unit test project. The functionality already exists. You still have to do that with the new method.
I want simple, easy and usable without having to spend a load of time configuring. Instead we got a steaming turd because it feels like their ASP.Net Core developers don't seem to get what DI is for. I feel like the ASP.Net team has been getting far too opinionated and often shows a lack of understanding about how swathes of their developers use their product.
This in particular reminds me a lot of the WCF and WWF over-engineering and over-reliance on lots of config and voodoo magic and rain-dances when it went wrong.
That just felt insane!
There is a case for a mutable config, but it should be a separate class, and still wouldn't need to be a service.
I'm currently in a situation where I'm on the fence about two different ways to access my database (both are terrible options). Both db drivers are very different, and require very different code. However, they both return the same models. By defining an interface and injecting one service or the other in to my controllers, I can quite effectively and painlessly switch between the two. Why not a singleton? Because with one of the drivers it's better to build a new one on every request.
Back to injecting configuration... I actually use this database access service across two different ASP.NET Core projects. The configuration manager has a nice way to extract only parts of the configuration in to a model. Combine with DI and I can easily split my configuration models between these projects without a shared config model.
Sure, there's other ways to skin the cat. I'm not saying DI is the only way, but I do find it an effective method and certainly not a "nasty language hack".
There's absolutely no reason you have to do DI in C#. C# only recently had native DI introduced.
The only reason to use DI is to be able to inject mocks. It's the only reason. Ruby, for example, can do this without DI. It's literally making bad code just to be able to test it.
It's a language bug that DI is needed.
There are some who believe DI somehow magically makes your code less coupled, but go take a look at any client project in the wild and that's immediately a laughable and indefensible position. If anything, DI makes it worse. I've seen classes with 20 odd service injected, which each all have their own 10 or 15 classes injected, making that class coupled to something like 70-100 other classes.
And DI makes it even harder to pick them apart.
If you don't like DI and you're using C#, you're gonna be swimming upstream. It's been the C# dogma for many years. You sound like you'd be much happier away from C#.
Me too, and it's easy to take this approach way too far to where you're basically recreating your own MVC architecture in some pseudo-service layer that you're not 100% sure how it even gets executed in the first place.
But I'll admit for things you should be doing on 99.9% of calls (e.g. validating the model) it's nice to have it done one place and one place only.
// return bad request, add errors, or redirect somewhere
An ActionFilterAttribute might be a good approach, but in a real application I'd want two things: to apply it explicitly at the controller or method level rather than globally, and to pass it arguments to tell it what to do when there is an error. One of the arguments should allow me to call a response handler method, even if the only way to do that is to pass a string with the method name and use reflection to find and invoke it.I feel the same way as you however I've found that when working on a large code base with a lot of junior developers or contractors who lack qualities like discipline, attention to detail, consistency, and convention; it's often easier to just wire up all of the important parts behind the scenes.
It's also useful when you're managing a number of projects that are primarily maintained by junior developers and you only drop in occasionally to do the heavy lifting.
One pet peeve is the unnecessary usage of 'IHttpContextAccessor'. I prefer to register the 'HttpContext' as a scoped dependency, and then use it directly wherever possible (i.e in transitive and scoped dependencies).
The reason being theres nothing to help the developer not accidentally hold onto the 'HttpContext' from 'IHttpContextAccessor' for too long, where as it's harder to misuse the scoped 'HttpContext' now that asp net core 2 checks the dependency graph for captive dependencies.
So in this example if the 'ApplicationUserManager' was accidentally changed to a singleton at startup then it would silently cache the first user and serve it for all requests. If you depend on the 'HttpContext' directly that's not possible for any of it's dependents as long as the 'HttpContext' itself is registered properly.
seriously?
I don't believe it is useful to save a trip to the database, the database is there for a reason.
ex.InnerException.Message.IndexOf("duplicate", StringComparison.OrdinalIgnoreCase) > -1
Good idea would also be to check for existence of Inner exception at all ex.InnerException?.Message.IndexOf("duplicate", StringComparison.OrdinalIgnoreCase) > -1
But I would just live without that check to prevent magical behavior.I done a fair bit of performance problem fixing now in my career and I have never seen a .ToLower be the cause of any client's performance problems. Or really any string manipulation opinion like stringbuilder vs += and all that nonsense.
From C# via the CLR: Important If you want to change the case of a string's characters before performing an ordinal comparison, you should use String’s ToUpperInvariant or ToLowerInvariant method. When normalizing strings, it is highly recommended that you use ToUpperInvariant instead of ToLowerInvariant because Microsoft has optimized the code for performing uppercase comparisons. In fact, the FCL internally normalizes strings to uppercase prior to performing case insensitive comparisons. We use ToUpperInvariant and ToLowerInvariant methods because the String class does not offer ToUpperOrdinal and ToLowerOrdinal methods. We do not use the ToUpper and ToLower methods because these are culture sensitive.
And from MS
CA1308: https://docs.microsoft.com/en-gb/visualstudio/code-quality/c...
https://github.com/mysql-net/MySqlConnector/blob/master/src/...
Check that MySqlException.Number == 1062 for Duplicate entry.
Add one check per DBMS that you use.
Also, this Cierge thing is great! Didn't even know something like that exists (in asp.net core, except IdentityServer, which more like enterprise solution)
It's one of those magic features that ultimately hides too much.
Far too much can go wrong and the error get silently swallowed, plus having a parameterless constructor is rare for complex objects. So you end up with a model missing properties you need filled in if there's a validation error, extra work to make sure it's properly populated that should have happened in the constructor, which means code duplication.
Totally get the benefit of DTOs for create / update, but it feels like an unnecessary step in read operations.
Two things:
1.) ReSharper full solution analysis should bring those errors up. I wouldn't leave it on all the time, but flipping it on/off occasionally works wonders.
2.) Even without that, turn on the option to compile your views when you build. It's not super graceful (you'll have to fix one error at a time), but at least you won't accidentally ship a broken view!
But what if you expose domain model to the view, and view changes some property? Are you 100% it will not be saved to db?
If you change one field in a db, do you have to change domain model and UI also?
Important concept is Persistence Ignorance, and how "far" are you willing to go with decoupling application layer from database and domain model. It really depends on your app, and there's no "one correct way of doing this".
Something else to consider is that maybe your MVC project is not your ONLY consumer of your application services. Now it makes a lot more sense to decouple things and maintain that extra layer.
Also the ViewModel is tightly bound to the view, if you want to change the view in clever ways that should not change the domain models.
My security sense is tingling. Sounds like a convenient opening for me to finagle your app into an unexpected state (by crafting requests that trigger update failures, interrupting execution at targeted points).
Checking first and failing can still be problematic without the right database isolation setting.
Better to try it and catch the database error (which might include another unique constraint other than primary key... depending on db design).
TL;DR Yes I recommend it, lately Microsoft has been developing really cool stuff, and I doubt they will disappoint us, at least in the short to medium term.
public class ApplicationUserManager<TUser>
Plus you could just have IHttpContextAccessor injected instead and have extension methods off the ClaimsPrincipal to retrieve specific claims
but ... horses for courses I guess