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

Dependency injection topic and sample updates #7412

Merged
merged 18 commits into from
Jul 28, 2018
Merged

Conversation

guardrex
Copy link
Collaborator

@guardrex guardrex commented Jul 2, 2018

Addresses #5495
Fixes #7208
Fixes #6331
Addresses #7700

Dependency injection topic
Repository pattern topic

  • Upgrade 1.0 MVC sample to 1.1.
  • Shiny new 2.1 RP sample. 🎉
  • The usual suspects :suspect: ... grammar, style, API links, tabs out, etc.
  • Replace the repository pattern example of DI with a simpler example. Move the repository pattern to its own topic. Repository pattern is overkill in the DI topic, and we aren't using it in this doc set per the guidance of @danroth27. However, it's a great pattern, so I hate to lose the content. It composes well into it's own topic with its own little sample app cross-linked back and forth to the DI topic. The DI example I use here is easy to grok and makes it simple to ........
  • Employ the ⚡ Wasson Gambit™️ ⚡ ... i.e., take the time and space to explain DI with a clear example showing why hardcoded dependencies are bad before explaining why DI is better. Tip of the 🎩 to @MikeWasson for https://docs.microsoft.com/aspnet/web-api/overview/advanced/dependency-injection#what-is-dependency-injection. I looked at quite a few DI explanations around the Net, and I think your approach is THE BEST. You may be surprised how many DI explanations were apparently written for 🔪 Halloween readings to frighten young children! 🎃
  • I retain the "Operations" services example from the current topic, but I modify the explanations a bit to (hopefully) make it a bit clearer what it's showing, especially the OperationService.

@guardrex
Copy link
Collaborator Author

guardrex commented Jul 2, 2018

@Rick-Anderson @scottaddie @rachelappel I think it best if you look this over prior to engineering. I'm taking this in a new direction with a major overhaul to the introduction and how DI is explained. It's probably best for you to say if you like it ... or issue a round of 💀 Rex death threats 💀 😄 ... before engineering takes a look.


By [Steve Smith](https://ardalis.com/) and [Scott Addie](https://scottaddie.com)
ASP.NET Core supports the dependency injection (DI) software design pattern, which is a technique for achieving [Inversion of Control (IoC)](http://deviq.com/inversion-of-control/) between classes and their dependencies.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

http --> https


Note that `CharacterRepository` requests an `ApplicationDbContext` in its constructor. It's not unusual for dependency injection to be used in a chained fashion like this, with each requested dependency in turn requesting its own dependencies. The container is responsible for resolving all of the dependencies in the graph and returning the fully resolved service.
`MyDependency` requests an [ILogger<TCategoryName>](/dotnet/api/microsoft.extensions.logging.ilogger-1) in its constructor. It's not unusual to use dependency injection in a chained fashion with each requested dependency in turn requesting its own dependencies. The container resolves the dependencies in the graph and returns the fully resolved service. The collective set of dependencies that must be resolved is typically referred to as a *dependency tree*, *dependency graph*, or *object graph*.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a comma after "in a chained fashion".

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No comma between prepositional phrases. However, let's break the sentence here. That's really better in this case for localization anyway.

It's not unusual to use dependency injection in a chained fashion. Each requested dependency in turn requests its own dependencies.

Note that `CharacterRepository` requests an `ApplicationDbContext` in its constructor. It's not unusual for dependency injection to be used in a chained fashion like this, with each requested dependency in turn requesting its own dependencies. The container is responsible for resolving all of the dependencies in the graph and returning the fully resolved service.
`MyDependency` requests an [ILogger<TCategoryName>](/dotnet/api/microsoft.extensions.logging.ilogger-1) in its constructor. It's not unusual to use dependency injection in a chained fashion with each requested dependency in turn requesting its own dependencies. The container resolves the dependencies in the graph and returns the fully resolved service. The collective set of dependencies that must be resolved is typically referred to as a *dependency tree*, *dependency graph*, or *object graph*.

`IMyDependency` and `ILogger<TCategoryName>` must be registered in the service container. `IMyDependency` is registered in `Startup.ConfigureServices`. `ILogger<TCategoryName>` is registered by the logging abstractions infrastructure, so it's a [framework-provided-service](#framework-provided-services) registered by default by the framework.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove the dash between "provided" and "service".


Entity Framework contexts should be added to the services container using the `Scoped` lifetime. This is taken care of automatically if you use the helper methods as shown above. Repositories that will make use of Entity Framework should use the same lifetime.
In the following example class from the sample app, the `IMyDependency` instance is requested and used to call the service's `WriteMessage` method:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

example class --> class


**Singleton**

Singleton lifetime services are created the first time they're requested (or when `ConfigureServices` is run if you specify an instance there) and then every subsequent request will use the same instance. If your application requires singleton behavior, allowing the services container to manage the service's lifetime is recommended instead of implementing the singleton design pattern and managing your object's lifetime in the class yourself.
Singleton lifetime services are created the first time they're requested (or when `ConfigureServices` is run if an instance is specified there) and then every subsequent request uses the same instance. If the app requires singleton behavior, allowing the service container to manage the service's lifetime is recommended. Don't implement the singleton design pattern and provide user code to manage the object's lifetime in the class.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a comma before "if an instance is".

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Singleton lifetime services are created:

  • The first time

Or some other way to keep sentence under 20 words.

services.AddSingleton<Service3>(new Service3());
services.AddSingleton(new Service3());
}
```

> [!NOTE]
> In version 1.0, the container called dispose on *all* `IDisposable` objects, including those it didn't create.
> In ASP.NET Core 1.0, the container calls dispose on *all* `IDisposable` objects, including those it didn't create.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Display this note only when the 1.0 version of the docset is selected.


Finally, configure Autofac as normal in `DefaultModule`:
3. Configure Autofac normally in `DefaultModule`:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm confused by the word "normally" in this context. Is it needed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm confused by the word "normally" in this context. Is it needed?

I think it just fell under "conversational tone." I'm removing it now.


At runtime, Autofac will be used to resolve types and inject dependencies. [Learn more about using Autofac and ASP.NET Core](http://docs.autofac.org/en/latest/integration/aspnetcore.html).
At runtime, Autofac is used to resolve types and inject dependencies. To learn more about using Autofac with ASP.NET Core, see the [Autofac documentation](http://docs.autofac.org/en/latest/integration/aspnetcore.html).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

http --> https


## Additional resources

* [Dependency injection into views](xref:mvc/views/dependency-injection)
* [Dependency injection into controllers](xref:mvc/controllers/dependency-injection)
* [Dependency injection in requirement handlers](xref:security/authorization/dependencyinjection)
* [Repository pattern](xref:fundamentals/repository-pattern)
* [Application Startup](xref:fundamentals/startup)
* [Test and debug](xref:test/index)
* [Factory-based middleware activation](xref:fundamentals/middleware/extensibility)
* [Writing Clean Code in ASP.NET Core with Dependency Injection (MSDN)](https://msdn.microsoft.com/magazine/mt703433.aspx)
* [Container-Managed Application Design, Prelude: Where does the Container Belong?](https://blogs.msdn.microsoft.com/nblumhardt/2008/12/26/container-managed-application-design-prelude-where-does-the-container-belong/)
* [Explicit Dependencies Principle](http://deviq.com/explicit-dependencies-principle/)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

http --> https


## Additional resources

* [Dependency injection into views](xref:mvc/views/dependency-injection)
* [Dependency injection into controllers](xref:mvc/controllers/dependency-injection)
* [Dependency injection in requirement handlers](xref:security/authorization/dependencyinjection)
* [Repository pattern](xref:fundamentals/repository-pattern)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Prefer the xref:uid pattern for these additional resources links.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😕 They do. What do you mean?

Copy link
Member

@scottaddie scottaddie Jul 2, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, my comment didn't display as expected. I meant use <xref:mvc/views/dependency-injection>, for example. The link text is applied for you.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIRC the decision that was made back in Tom days was to split the "ASP.NET Core" out by specifying the shorter manual titles. Looks like a change of direction was settled upon since then. Are we doing this for all cross-links or just for Additional resources section links?

Copy link
Contributor

@Rick-Anderson Rick-Anderson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@guardrex I seem to remember there is a trick to see the Console.WriteLine( output from VS and from CLI.

From VS the output is a needle in the haystack. Is there a way to remove the default debug output so you can see these messages? If you add that to the readme or the doc. If you add it to the readme mention in the doc the readme shows how to view the messages.


In this case, both `ICharacterRepository` and in turn `ApplicationDbContext` must be registered with the services container in `ConfigureServices` in `Startup`. `ApplicationDbContext` is configured with the call to the extension method `AddDbContext<T>`. The following code shows the registration of the `CharacterRepository` type.
If a service's constructor requires a primitive, such as a `string`, this can be injected by using [configuration](xref:fundamentals/configuration/index) and the [options pattern](xref:fundamentals/configuration/options).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this can be injected
What is this?
The primative can be .. or The string can be


Entity Framework contexts should be added to the services container using the `Scoped` lifetime. This is taken care of automatically if you use the helper methods as shown above. Repositories that will make use of Entity Framework should use the same lifetime.
In the following example class from the sample app, the `IMyDependency` instance is requested and used to call the service's `WriteMessage` method:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the following example class from the sample app


ASP.NET services can be configured with the following lifetimes:
It's important to choose an appropriate lifetime for each registered service. ASP.NET Core services can be configured with the following lifetimes:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Services must be registered with the appropriate lifetime. Now explain why. You can't just say important without explaining why. This is a good question to get PU help on.

Copy link
Collaborator Author

@guardrex guardrex Jul 2, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree. That text is a holdover from Line 103 of the original topic ...

The AddTransient method is used to map abstract types to concrete services that are instantiated separately for every object that requires it. This is known as the service's lifetime, and additional lifetime options are described below. It's important to choose an appropriate lifetime for each of the services you register. Should a new instance of the service be provided to each class that requests it? Should one instance be used throughout a given web request? Or should a single instance be used for the lifetime of the application?

AFAIK a main one must relate to isolation and possibly reducing resource demands .... I need PU to help if the reasons go beyond that. I'll ask if they don't remark on it.

Copy link
Member

@scottaddie scottaddie Jul 2, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Explain what's meant by lifetime.

@guardrex
Copy link
Collaborator Author

guardrex commented Jul 2, 2018

From VS the output is a needle in the haystack. Is there a way to remove the default debug output so you can see these messages? If you add that to the readme or the doc. If you add it to the readme mention in the doc the readme shows how to view the messages.

Yes, there is a way to filter ... but if you're referring to the inline code block in the intro where I show Console.WriteLine, that's for display-only purposes in the topic. The sample app doesn't implement it.

I don't want to show a dependency that calls for another dependency at this point in the topic. I do want to swap out the Console.WriteLine for the ILogger when I show how DI works. This is why I left the empty constructor in the inlined code block ... I add the ILogger there when DI is shown. I want that first dependency class example to look a lot like the upcoming version of it for the DI explanation.

@guardrex
Copy link
Collaborator Author

guardrex commented Jul 2, 2018

@Rick-Anderson I'll let you call the right engineering cat (or cats) when the time comes.

@guardrex
Copy link
Collaborator Author

guardrex commented Jul 2, 2018

btw @Rick-Anderson I looked that up just in case we need it. An example goes back to the first version of the LoggerMessage topic. This will do the trick if we ever need to filter console logging to get rid of the spew ...

.ConfigureLogging((hostingContext, logging) =>
{
    logging.ClearProviders();
    logging.AddConfiguration(hostingContext.Configuration.GetSection("Logging"));
    logging.AddFilter("Microsoft.EntityFrameworkCore.Update", LogLevel.None);
    logging.AddFilter("Microsoft.EntityFrameworkCore.Infrastructure", LogLevel.None);
    logging.AddFilter("Microsoft.AspNetCore.Mvc.RedirectToRouteResult", LogLevel.None);
    logging.AddFilter("Microsoft.AspNetCore.Mvc.RazorPages.Internal.PageActionInvoker", LogLevel.None);
    logging.AddFilter("Microsoft.AspNetCore.StaticFiles.StaticFileMiddleware", LogLevel.None);
    logging.AddConsole();
})


ASP.NET services can be configured with the following lifetimes:
It's important to choose an appropriate lifetime for each registered service. ASP.NET Core services can be configured with the following lifetimes:
Copy link
Member

@scottaddie scottaddie Jul 2, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Explain what's meant by lifetime.


First, install the appropriate container package(s):
The built-in service container is meant to serve the basic needs of the framework and most consumer apps built on it. However, developers can replace the built-in container with their preferred container. The `Startup.ConfigureServices` method typically returns `void`. If the method's signature is changed to return an [IServiceProvider](/dotnet/api/system.iserviceprovider), a different container can be configured and returned. There are many IoC containers available for .NET. In the following example, the [Autofac](https://autofac.org/) container is used:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirm with engineering, but I seem to remember something related to this wasn't possible until ASP.NET Core 1.1. As noted in the Autofac docs, "ASP.NET Core 1.1 introduced the ability to have strongly typed container configuration." Also, I have a personal 2.0 app using Autofac with a return type of void in Startup.ConfigureServices.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok ... will do.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ConfigureContainer looks like it was 1.1, but returning IServiceProvider from ConfigureServices was 1.0, so I think we're ok as far as this passage is concerned. aspnet/Hosting#91

@guardrex
Copy link
Collaborator Author

guardrex commented Jul 2, 2018

Open subjects for PU comment:


The `ConfigureServices` method in the `Startup` class is responsible for defining the services the application will use, including platform features like Entity Framework Core and ASP.NET Core MVC. Initially, the `IServiceCollection` provided to `ConfigureServices` has the following services defined (depending on [how the host was configured](xref:fundamentals/host/index)):
The class creates and directly depends on the `MyDependency` instance. However, it's a bad practice to hardcode the dependency in this way for the following reasons:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

However, it's a bad practice to hardcode the dependency in this way for the following reasons:

We should avoid bad. Hardcode is jargon and should be avoided. Maybe something like

Code dependencies (such as the previous example) are problematic and should be avoided for the following reasons:

* `IServiceProvider`
* [ActivatorUtilities](/dotnet/api/microsoft.extensions.dependencyinjection.activatorutilities)

When services are resolved by `IServiceProvider` or `ActivatorUtilities`, constructor injection requires a *public* constructor. Otherwise, the app throws an [InvalidOperationException](/dotnet/api/system.invalidoperationexception):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Otherwise, the app throws an InvalidOperationException:

Delete all that stuff, TMI. It bloats the article. Fantastic error message, no need to add all those details.
+
+> A suitable constructor for type '<TYPE>' couldn't be located. Ensure the type is concrete and services are registered for all parameters of a public constructor.


Constructors can accept arguments that aren't provided by dependency injection, but these must support default values.

The following code throws an `InvalidOperationException` because the runtime is unable to resolve the `item` string:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you should remove all that code, just give them the rule. The valid and invalid code is nonsense. There's no reason to do that. The rule is all you need.

Unless you have reasonable code, don't show it.


## Lifetime and registration options

To demonstrate the difference between the lifetime and registration options, consider the following interfaces that represent tasks as an operation with a unique identifier, `OperationId`. Depending on how the lifetime of a service is configured, the container provides either the same or a different instance of the service to the requesting class.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Depending on how the lifetime of a service is configured, the container provides either the same or a different instance of the service to the requesting class.

Can you say
The OperationId depends on the service lifetime. X, Y, Z return a new instance of OperationId. M, X returns ...

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Idk about this ... the statement is accurate and the extensions are both obvious and previously covered in the Service lifetimes section just above this section.

I would like to clarify it to ...

Depending on how the lifetime of an operations service is configured for the following interfaces, the container provides either the same or a different instance of the service when requested by a class:

... which makes it a hair clearer that the following interfaces will be used to create services shortly and that service lifetime registrations will dictate receiving a new or existing instance. The naming of the interfaces is obvious on which one goes with which lifetime registration.

@Rick-Anderson
Copy link
Contributor

Rick-Anderson commented Jul 2, 2018

@scottaddie @guardrex for some reason I can't find the following markup From the review page. I can see it in the review URL and when I edit the content.

## Scope validation

When the app is running in the Development environment on ASP.NET Core 2.0 or later, the default service provider performs checks to verify that:

--- end of stuff I can't find

Question for @scottaddie
Should we change this to

:::moniker 2.0 >
When the app is running in the Development environment on ASP.NET Core 2.0 or later, the default service provider performs checks to verify that:


::: moniker-end

These interfaces are implemented using a single class, `Operation`, which accepts a `Guid` in its constructor or uses a new `Guid` if none is provided:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The interfaces are all implemented in the Operation class. The Operation constructor takes a guid , if supplied; otherwise it creates a new guid.

Something like that. Shorter sentences. More terse.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or maybe something like generates a guid if one is not supplied? It's a bit long now.


Services can be registered with the container in several ways. We have already seen how to register a service implementation with a given type by specifying the concrete type to use. In addition, a factory can be specified, which will then be used to create the instance on demand. The third approach is to directly specify the instance of the type to use, in which case the container will never attempt to create an instance (nor will it dispose of the instance).
An `OperationService` is registered that depends on each of the other `Operation` types. `OperationService` makes it clear if transient services are created when requested or merely mirroring the `OperationsId` of the `IOperationTransient` service:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OperationService makes it clear if transient services are created when requested or merely mirroring the OperationsId of the IOperationTransient service:

Where does it make it clear. I don't see that. What does or merely mirroring the OperationsId of the IOperationTransient service: mean?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where does it make it clear. I don't see that. What does or merely mirroring the OperationsId of the IOperationTransient service: mean?

I'll come back to this one in a bit. It's going to take a few minutes to work it out.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where does it make it clear. I don't see that. What does or merely mirroring the OperationsId of the IOperationTransient service: mean?

I did a reboot on this section. We'll have to see tho ... is it an improvement 😄 🎉 or a white line nightmare 👦 🔫.

services.AddMvc();

// Register application services.
services.AddScoped<ICharacterRepository, CharacterRepository>();
services.AddScoped<IMyDependency, MyDependency>();
services.AddTransient<IOperationTransient, Operation>();
services.AddScoped<IOperationScoped, Operation>();
services.AddSingleton<IOperationSingleton, Operation>();
services.AddSingleton<IOperationSingletonInstance>(new Operation(Guid.Empty));
services.AddTransient<OperationService, OperationService>();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a code comment here

// OperationService depends on each of the other Operation types.
That helps split them up.


::: moniker range="<= aspnetcore-2.0"

To demonstrate the object lifetimes within and between individual requests to the app, the sample app includes an `OperationsController` that requests each kind of `IOperation` type as well as the `OperationService`. The `Index` action sets the services into the `ViewBag` for display of the service's `OperationId` values:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Too long, split up - maybe list.
as well as is banned in this repo. It's dead wood.


::: moniker-end

Two requests are made with the following result:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The following images show two requests:

* *Scoped* objects are the same within a request but different across requests.
* *Singleton* objects are the same for every object and every request regardless of whether an `Operation` instance is provided in `ConfigureServices`.

## Resolve a scoped service within the app scope
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Heading is too abstract. What about Calling services from main? or something like that. Then import a code snippet from https://docs.microsoft.com/en-us/aspnet/core/tutorials/razor-pages/sql?view=aspnetcore-2.1#add-the-seed-initializer or https://docs.microsoft.com/en-us/aspnet/core/data/ef-rp/intro?view=aspnetcore-2.1&tabs=visual-studio

YOu could even tell them what's going on

Get a DB context instance from the dependency injection container.
Call the seed method, passing to it the context.
Dispose the context when the seed method completes.

Copy link
Collaborator Author

@guardrex guardrex Jul 2, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Heading is too abstract.

Fixed.

import a code snippet from ... Get a DB context instance from the dependency injection container ...

We already have one tho. This is about more than just dB contexts. I don't want a reader to get too married to the idea that that's all this is for. Let me try a version that links to that section as a further example. If it isn't well received, I can swap it out completely on a future commit.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh ... wait ... I don't think we can do that at all. Seeding is entering a new world order here with 2.1 ... we have #7342 to work it over.


You should design your services to use dependency injection to get their collaborators. This means avoiding the use of stateful static method calls (which result in a code smell known as [static cling](http://deviq.com/static-cling/)) and the direct instantiation of dependent classes within your services. It may help to remember the phrase, [New is Glue](https://ardalis.com/new-is-glue), when choosing whether to instantiate a type or to request it via dependency injection. By following the [SOLID Principles of Object Oriented Design](http://deviq.com/solid/), your classes will naturally tend to be small, well-factored, and easily tested.
Design services to use dependency injection to obtain their dependencies. Avoid the use of stateful, static method calls (a bad practice known as [static cling](https://deviq.com/static-cling/)) and the direct instantiation of dependent classes within services. Direct instantiation "glues" the code to a particular implementation. It may help to remember the phrase, [New is Glue](https://ardalis.com/new-is-glue), when choosing whether to instantiate a type or to request it via dependency injection. By following the [SOLID Principles of Object Oriented Design](https://deviq.com/solid/), app classes naturally tend to be small, well-factored, and easily tested.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@scottaddie can you help me come up with a replacement for Avoid and bad? It should be stronger than avoid.

A best practice is to :

  • Design services to use dependency injection to obtain their dependencies.
  • Not us stateful, static method calls (known as static cling).
  • Not use direct instantiation of dependent classes within services


You should design your services to use dependency injection to get their collaborators. This means avoiding the use of stateful static method calls (which result in a code smell known as [static cling](http://deviq.com/static-cling/)) and the direct instantiation of dependent classes within your services. It may help to remember the phrase, [New is Glue](https://ardalis.com/new-is-glue), when choosing whether to instantiate a type or to request it via dependency injection. By following the [SOLID Principles of Object Oriented Design](http://deviq.com/solid/), your classes will naturally tend to be small, well-factored, and easily tested.
Design services to use dependency injection to obtain their dependencies. Avoid the use of stateful, static method calls (a bad practice known as [static cling](https://deviq.com/static-cling/)) and the direct instantiation of dependent classes within services. Direct instantiation "glues" the code to a particular implementation. It may help to remember the phrase, [New is Glue](https://ardalis.com/new-is-glue), when choosing whether to instantiate a type or to request it via dependency injection. By following the [SOLID Principles of Object Oriented Design](https://deviq.com/solid/), app classes naturally tend to be small, well-factored, and easily tested.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Direct instantiation "glues" the code to a particular implementation.

How about couples? Need to avoid slang and jargon.

It may help to remember the phrase, New is Glue, when choosing whether to instantiate a type or to request it via dependency injection.


With regards to data access specifically, you can inject the `DbContext` into your controllers (assuming you've added EF to the services container in `ConfigureServices`). Some developers prefer to use a repository interface to the database rather than injecting the `DbContext` directly. Using an interface to encapsulate the data access logic in one place can minimize how many places you will have to change when your database changes.
With regard to data access specifically, an Entity Framework Core `DbContext` can be injected into Razor Pages page models and MVC controllers. However, some developers prefer to use a repository interface to the database rather than injecting the database context directly. Using an interface to encapsulate the data access logic in one place can minimize code changes when the database changes. For more information, see [Repository pattern](xref:fundamentals/repository-pattern).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nuke this entire paragraph.


Finally, configure Autofac as normal in `DefaultModule`:
> [!NOTE]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove note chrome. How about
To use a 3rd party container, Startup.ConfigureServices must return IServiceProvider

@Rick-Anderson
Copy link
Contributor

Avoid might be OK here, it's used extensively and I don't have a great alternative.

@scottaddie
Copy link
Member

scottaddie commented Jul 2, 2018

@Rick-Anderson The Scope validation section is there in the GitHub diff page. You'll need to click the icon by the line numbers to see it:
image

@guardrex I like Rick's suggestion of only displaying that 1st Scope validation paragraph for 2.0+.

@guardrex guardrex requested a review from davidfowl July 6, 2018 14:19
@guardrex
Copy link
Collaborator Author

guardrex commented Jul 6, 2018

@davidfowl This is an important set of updates for the DI topic. Do you want to 🔪 carve me up 🦃?

@guardrex guardrex requested a review from pakrym July 9, 2018 22:49
@guardrex
Copy link
Collaborator Author

guardrex commented Jul 9, 2018

@pakrym, @davidfowl recommended you TAL at this, too.

This is a fairly heavy rewrite to simplify the Programming 101 explanation of DI. If it's ultimately determined that this topic should launch right into DI with something like EF/RazorPagesMovie as the example, then I propose we move the intro that I have here (based on the excellent approach for new devs taken by @MikeWasson) into an index topic under a new DI node, then have this DI topic (as a more advanced reference topic) live under the DI node.

The rest of this is mostly sample updates and guidelines adherence. I split the Repository Pattern out into its own topic with its own sample app. We don't use or explicitly suggest the pattern, but imo it's good to keep.


In this case, both `ICharacterRepository` and in turn `ApplicationDbContext` must be registered with the services container in `ConfigureServices` in `Startup`. `ApplicationDbContext` is configured with the call to the extension method `AddDbContext<T>`. The following code shows the registration of the `CharacterRepository` type.
If a service's constructor requires a primitive, such as a `string`, the primitive can be injected by using [configuration](xref:fundamentals/configuration/index) and the [options pattern](xref:fundamentals/configuration/options).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would rephrase it somehow or add a sample. It's not entirely clear what should I do with the string parameter.


When services are resolved by `IServiceProvider` or `ActivatorUtilities`, constructor injection requires a *public* constructor.

When services are resolved by `ActivatorUtilities`, constructor injection requires that only one applicable constructor exist. Constructor overloads are supported, but only one overload can exist whose arguments can all be fulfilled by dependency injection.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs more info where and why ActivatorUtilities is used

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pakrym I see the API remarks and a few code comments here and there (e.g., Hosting repo for StartupLoader.LoadMethods), which don't seem quite adequate to just drop in here. I'd prefer if you or someone from engineering write a paragraph for this or even just supply a handful of sentences that I can wordsmith into a paragraph.

There's probably a need for an advanced DI topic that goes beyond the basics laid out in this topic. I'm aware of @stevejgordon's Expand DI documentation with additional extension method overloads (aspnet/Docs #6312), but it feels to me like we need something between this topic and those. I propose we open a new issue for it tho ... we're swamped at the moment.


::: moniker-end

An `OperationService` is registered that depends on each of the other `Operation` types. When `OperationService` is requested via dependency injection, it receives either a new instance of each service or an existing instance based on how the dependent service is registered.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

based on the lifetime of dependent service


* Avoid static access to services.

* Avoid service location in your application code.
* Avoid service location in app code.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

using service locator pattern

React to feedback

Updates
@guardrex
Copy link
Collaborator Author

@pakrym See if the examples I added are sane ...

  • Avoid static access to services (for example, [IApplicationBuilder.ApplicationServices](/dotnet/api/microsoft.aspnetcore.builder.iapplicationbuilder.applicationservices)).

  • Avoid using the service locator pattern (for example, [IServiceProvider.GetService](/dotnet/api/system.iserviceprovider.getservice)).

  • Avoid static access to HttpContext.

Is an example of the last one RequestServices?

@pakrym
Copy link
Contributor

pakrym commented Jul 11, 2018

They are not entirely correct, a bit better example of "static access to HttpContext" is using IHttpContextAccessor. While being a service it's implemented as AsyncLocal and has a lot of similarities with static HttpContext.

Using IApplicationBuilder.ApplicationServices on the other hand is fine when appropriate.

@guardrex
Copy link
Collaborator Author

@pakrym How about making it more specific for ApplicationServices?

  • Avoid static access to services (for example, statically-typing [IApplicationBuilder.ApplicationServices](/dotnet/api/microsoft.aspnetcore.builder.iapplicationbuilder.applicationservices) for use elsewhere).

... or do you have an example ... something devs commonly do that's badbadnot good.

@guardrex
Copy link
Collaborator Author

@pakrym I modified your description of ActivatorUtilities for the topic on the last commit.

@pakrym
Copy link
Contributor

pakrym commented Jul 19, 2018

@guardrex sorry it takes me so long to review was a very busy week. I'll read through everything again Monday and finish the review.

@scottaddie scottaddie mentioned this pull request Jul 24, 2018
@guardrex
Copy link
Collaborator Author

Thanks @pakrym!

@davidfowl ... do you still want to take a look?

@guardrex
Copy link
Collaborator Author

@Rick-Anderson @scottaddie Anything else on this one?

@Rick-Anderson Rick-Anderson merged commit 940ea3c into master Jul 28, 2018
@Rick-Anderson Rick-Anderson deleted the guardrex/di-update branch July 28, 2018 22:49
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

Successfully merging this pull request may close these issues.

4 participants