Hacker News new | comments | show | ask | jobs | submit login
A todo list for new ASP.NET Core projects (biarity.gitlab.io)
150 points by biarity 9 months ago | hide | past | web | favorite | 63 comments



One of the cool things about ASP.NET is how many different ways there are to get things done. There is a ton of stuff in there, and you can pick and choose the bits that make sense for your project.

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.


> 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.

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.


> I tend to disagree with the author's approach of pushing routine stuff out to the realm of "magically happens behind the scenes"

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.


The drawback to validating the model in one place only, as shown, is that there is only one response for validation errors. Sure, it's got the details of the error that will vary, but it's ALWAYS sending a 400 error. Even the example of the 'usual' way includes a comment showing multiple possible responses:

    // 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 do feel the recent changes have moved to prescription rather than flexibiity though.

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.


Configuration can come from different sources, it can be reloaded, it can be lazy-loaded, there are so many different reasons why config is more like service and not a static variable. It has to be injected. Maybe you're using it in a way static access makes sense, but when you're designing generic Configuration solution, its whole another set of problems, requirements and constraints.


No it's not. We've had app config for the last 20 years. When you need mutable config put it in the db.

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 sort of agree that config is more of a service.. but I had the same reaction as the parent here that logging had to be injected!

That just felt insane!


I personally like DI because you can take one glance at the constructor and know what the class depends on, as well as the testability benefits. Can you expand more on what you don’t like about it?


Fundamentally DI is magic code. Where did that thing come from? How did it get initialised? How do I fix it when it breaks?

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.


If you don't like DI then I feel like C# in general won't be your cup of tea. As for the "only for testing" bit, I disagree.

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".


That's utter nonsense.

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.


I just gave you my reasons for using DI and you just ignored them, repeating that the only reason for DI is testing, even though my reasons are not for testing. That's utter nonsense.

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#.


Serious question: how do you test something without injection?

But yes I agree injecting configuration seems sub-optimal at best.


Read about Ruby and DHH's opinion on DI. That's how.

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.


In the case of the configuration manager, the test project would have its own underlying configuration file.


I don't see how injecting is crazy? You mean you find it crazy that you have to add an additional config parameter to your controller constructors? If that is what bothers you then you can create one base constructor where you inject the config and then have other classes derive from it...


It's simply not an appropriate thing to be injecting. App settings are that, settings for the app. They don't change on the fly, if they do change the app should restart, and it's definitely not something that should be injected.


I'm not quite sure what you mean. There is no ConfigurationManager used in Asp.NET Core. Are you talking about IConfiguration?

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).


Sigh. Of course I want the app to restart, it's configuration. Changing it on the fly and hoping the Devs coded defensively enough to handle mutating config is a recipe for disaster.

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.


I remember fondly "back in the old days" spending a couple of years doing Java servlets (for reference, this was right when JSPs were becoming a thing) where we implemented a nice compact MVC approach. The performance was very good at the time. When I moved over to ASP.NET I felt baffled by all of the heavy WebForms templating, but soon discovered that you could set up an almost identical framework as we had with servlets by implementing HttpHandlers and some light-weight views with all the ViewState stuff turned off (it was probably most similar to something like NancyFx now). I feel like many of the frameworks now are doing the same stuff that worked well 15 years ago, but it is fleshed out better. And of course there is a ton of open-source work, which was missing back then.


interestingly the plus points from your first sentence (which I agree are plus points) are the exact things that Scala is often criticised for.


Good read. The model validation is a great idea. Interestingly it will be done automatically in asp net core 2.1 for controllers marked with the new 'ApiControllerAttribute'.

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.


Usage of IHttpContextAccessor is official way on hot to access httpcontext. They (ms team) probably have a good reason why to expose IHttpContextAccessor and not httpContext directly.


Could you share a code sample of Startup.cs and a sample Controller or Service showing usage? Maybe in a GitHub gist?


> if (exception.InnerException.Message.ToLower().Contains("duplicate"))

seriously?


I know, I just couldn't find a cross-db solution since I can't seem to find any standardized error codes in the exception. Do you know of a better way of doing this?


I do not know of a good Cross DB solution. It may be more elegant to check specific exception types for the DBMSs that you do use. For example, MySQL would be:

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.


I would just delete that part if it can't be solved. Smaller issues with code are, that you create new string just to compare it (ToLower makes new instance), when you can achieve this just by using proper comparer such as

    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.


Yes, because the performance impact of a single string creation in an Exception handler is important.


I can't upvote this enough.

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.


If you want to get really picky you should be converting to uppercase and ToUpperInvariant in particular:

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...


Your code is harder to read and the performance implications are so trivial, a classic example of pre-optimization.


I agree with you. But I also think you're sacrificing clarity over brevity yourself by not expanding "pre-optimization" into "premature optimization" ;)


I myself really dislike that piece of code (searching in exception strings isn't secure for many reasons, for example someone could use this to reverse-engineer information about my database). But the only alternative that comes to mind is to check if an entity exists before attempting to create it - which I don't like because it leads to repetitive code in large projects (and I might forget to write checks). Any other ideas?


How about adding a "GetByID" function? The check is searching the entity, if NULL - save - if NOT NULL - update.

I don't believe it is useful to save a trip to the database, the database is there for a reason.


Yes, that is the usual way of doing it. However, the point of this post was to showcase some ways of reducing duplicate code in large projects. Doing it normally (by checking if the entity already exists) can get repetitive if you have many controllers/entities. If you know of a better way of doing this, I'd be happy to update my article :)


I'd use Regex.IsMatch() with an option for case-insensitive if I had to do that check. But I coded in Perl for 13 years, so regular expressions come naturally to me.


Why not just check the inner exception type instead of the contents? This seems more accurate than relying on the contents to contain a specific string.


Im definitely stealing some tricks from this list, very useful!

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)


Just spent the last 2 months of my life implementing IS4. So far its really powerful and flexible. I haven't done much research on Cierge but it looks like a neat out of the box solution for people who need a quick and easy solution for SSO.


I find myself using TryUpdateModel far more these days than relying on letting the controller bind automatically.

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.


I agree! I find this especially useful when I have to set things like a UserId associated with an entity or with more complex entities where some data comes from injected services. I know you can create a service that handles creating the entity as an abstraction, but that gets repetitive and adds complexity in large projects. I might actually add this my article :)!


I don't understand the item regarding validation. Whenever I actually care about validation, I am needing to use the 'IsValid' check to make some decisions. What does the attribute gain me?


You might have site where all the validation is supposed to be done on the client side, and the only way an invalid model reaches the server is if somebody's up to mischief.


up to mischief could mean broken or disabled javascript. As-in, maybe something else on your page (third-party ads) loads javascript with an error that stops execution, but your form is still submitable because you've coded it properly as a standard html5 form.


Thank you. That makes sense, although upon first reading I was worried by 'validation supposed to be done on the client side' ;)


General question for any pros on here, when querying for a read only report, is it standard practice to map from a viewModel/dto to a domain model? Seems a little counter productive to me (for reports) as viewModels will typically have extra concepts foreign to the actual domain model (eg pagination)?

Totally get the benefit of DTOs for create / update, but it feels like an unnecessary step in read operations.


If you don't have to map to DTO, or domain == dto, then its probably an overkill.

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".


I usually end up writing view models for reporting views because it condenses the type and amount data that is actually needed for the view, and (pet peeve) VS/Razor kind of don't work so well together in terms of notifying developers of errors while refactoring unless you actually have all of your views open in tabs in VS.


> VS/Razor kind of don't work so well together in terms of notifying developers of errors while refactoring unless you actually have all of your views open in tabs in VS.

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!


hudo's example is really good.

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.


> To do that, I created a DbUpdateExceptionHandler middleware that handles database-related exceptions globally (especially duplicate value inserts):

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).


Well the server was going to return an error anyways, instead of checking if the entity exists then trying to create it, you just go ahead and try to create it. This way if the entity doesn't exist then you've saved a trip to the database. Otherwise you return the same error. And given the server is a black box, how would you "interrupt execution"?


> checking if the entity exists then trying to create it,

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).


How is this specific to ASP.NET Core? Creating attributes for processing common functions isn't new with core.


Read on. Middleware providers are new to ASP.NET Core.


This comes at the best time possible. The last couple of days I have been contemplating giving this framework a try. Does anyone have any experience using ASP.NET Core under Linux and running it in production? If so, would you recommend it?


Me, well not really linux but his cousin from the Unix side, macOS. I have to admit, I was really doubtful about starting this project, I didn't want to lose time with obscure errors instead of working on the solution I wanted to implement. I'm a .NET developer, I use Windows at work and macOS at home. I wanted to start a side project, just for fun, I wanted to develop fast, since I don't have too much time in the afternoon to work on this side project, so I didn't want to lose too much time learning a totally new framework or language. Even though I was afraid I could found too much trouble in my learning process, I gave a try to ASP.NET Core in my mac, and I don't regret it, it has been awesome, I haven't found big issues preventing me to progress in my project. Working with VS Code has been really nice too, it's refreshing and clean, I almost haven't missed Visual Studio, actually I think I like it better (it feels kind of like a love affair). I'm still in the developing phase so I can't give you an opinion on deployment and testing my project in production, but now I'm confident that won't be a big issue.

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.


the generic parameter in this class is never used

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


I think your method is much cleaner actually. Just updated :)!


Thanks for reading these comments and updating your code! I personally appreciate the effort.


Nice read, cheers




Guidelines | FAQ | Support | API | Security | Lists | Bookmarklet | Legal | Apply to YC | Contact

Search: