
A todo list for new ASP.NET Core projects - biarity
https://biarity.gitlab.io/2018/02/08/asp-net-core-todo
======
jasonkester
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.

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

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

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

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

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

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

seriously?

~~~
biarity
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?

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

~~~
Dayshine
Yes, because the performance impact of a single string creation in an
_Exception handler_ is important.

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

~~~
LandR
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...](https://docs.microsoft.com/en-gb/visualstudio/code-
quality/ca1308-normalize-strings-to-uppercase)

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

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

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

~~~
biarity
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 :)!

------
tuespetre
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?

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

~~~
DougWebb
_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.

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

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

~~~
ctrl-j
> 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!

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

~~~
biarity
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"?

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

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

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

------
farresito
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?

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

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

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

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

------
jmkni
Nice read, cheers

