Jeremy Caney (28) [Avatar] Offline
Steven, Mark—

(I'm baaa-ack!)

This is largely more of the same, though the biggest feedback is on 4.4 Property Injection, which is worth calling out:
The bulk of this section reads like an anti-pattern. I understand why that is, since Property Injection solves a really specific problem, and outside of that it is frequently abused. Since this is in Chapter 4 DI Patterns, however, I'd recommend making the positive case for solving that specific problem. In addition, you might also consider adding a section called e.g., When not to use Property Injection, which would consolidate all of the potential problems and abuses associated with it.

More on that below.

Most of these would likely be picked up by you or your copy editors on a future pass, but I figured I'd flag them while I'm at it. These are minor typos that are easy to fix.

  • 4: In the first sentence, it should read, "Like all professionals, cooks have their own jargon that allows them to…"

  • 4: "Term" is misspelled as "rerm": "…we used the term reduction."

  • 4: In the bullet list, "declaring" should be "declare": "…allows a class to statically declare its required Dependencies."

  • 4.1.1: In the bullet list after the first Note, it should be "App.xaml.cs" instead of "App.xaml" (probably a bit pedantic, but an easy fix).

  • 4.1.1: After the second Note, it should read, "The advantage of this is that you can prevent…" and, later, "The downside of this approach, however, is that it isn't always very easy to do so."

  • 4.1.1: Unnecessary "your" in: "Separating your the presentation technology from the Composition Root might not be that beneficial…"

  • 4.1.3: In the paragraph after Listing 4.3, it should read, "The entire setup for the application is encapsulated in the CommerceControllerActivator class, which we'll show shortly."

  • 4.1.3: Shortly after Listing 4.3, it should read, "To enable wiring up MVC Controllers in the application…" instead of "To enable to wire up MVC Controllers in the application…"

  • 4.1.3: After Listing 4.4, it should read, "When MVC calls Create, we determine the Controller type and create the correct object graph based on this type."

  • 4.1.4: It should read e.g., "They see the use of DI as causing an explosion of dependencies…"

  • 4.1.4: It should read, "At first glance it indeed looks as if there is one more dependency…"

  • 4.1.4: At minimum, numbers nine and under should be spelled out; e.g., "…compared to Mary's application with 'only' three dependencies." (Beyond nine, style guides vary considerably, though I usually spell out any numbers that can be written with a single word, unless they're mixed with other numbers.)

  • 4.2.2: In the Extremely big object graphs callout, it should read, "Many of the classes in the system were huge, with way too many dependencies. Twenty or more dependencies…" (See previous note re: numbers.)

  • 4.2.2: In the last paragraph, "use" should be "uses", "usages" or "examples".

  • 4.3.2: In the fourth paragraph, it should be, "Imagine an add-in system for a graphical drawing application…"

  • 4.3.2: After Table 4.2, the following should read, "An important thing to note is, that, as we've seen with the add-in scenario, that the Dependency injected…" The second "that" is redundant.

  • 4.3.2: After Table 4.2, it should read, "In cases where the Dependency is an implementation detail to the caller…"

  • 4.3.2: In the Warning at the end of the section, "either" should be removed as it's a list of three options.

  • 4.3.2: In the Warning at the end of the section, it should read e.g., "A method is allowed to use the Dependency or pass it on, but should refrain from storing such Dependency." Alternatively, it could read, "A method should use the Dependency or pass it on, but refrain from storing such Dependency."

  • 4.3.4: The reference to Listing 4.7 should adhere to "Sentence case…" since it's at the beginning of the sentence. (Though, since all of the listings are Title Case, one might argue that their references should all be as well, regardless of where in a sentence they are located?)

  • 4.3.4: In Listing 4.13, "this.converter.ExchangeTo(…)" should be "converter.ExchangeTo(…)". converer is a method parameter, not a private field.

  • 4.3.4: In Listing 4.14, annotation #2, it should be, "The ICurrencyConverter Dependency is injected using Method Injection."

  • 4.3.4: In the last paragraph, it should read, "The next pattern in this chapter is Property Injection, which allows us to override a class's Local Default."

  • 4.4: In Figure 4.7, the caption should read, "…while using LocalDefault as the default implementation…"

  • 4.4.2: The first sentence of the third paragraph should read e.g., "In Chapter 1, we discussed many good reasons for writing code with loose coupling, thus isolating modules from each other."

  • 4.4.2: In the Open/Closed Principle callout, "without neither" should either be "with neither" or "without either"

  • 4.4.2: In the Open/Closed Principle callout, it should be, "nor its Dependency being aware of this."

  • 4.4.2: In the Open/Closed Principle callout, it should be, "As developers, it is your job…"

  • 4.4.2: In the Open/Closed Principle callout, the second-to-last line should read, "You should therefore strive to approach this ideal, by preventing sweeping changes to the system from happening regularly."

  • 4.4.2: In the Reusable Library callout, it should be "ecosystem" not "eco system".

  • 4.4.2: In the Reusable Library callout, it should be, "Although your Visual Studio solutions might contain projects that are reusable by multiple projects in the same solution, such projects are not considered to be reusable libraries."

  • 4.4.2: In the final Note, it should be, "…but that's a code smell, as we'll explain in Chapter 6."

  • 4.4.3: In the last sentence, it should be "With these BCL examples as an appetizer, let's move to a more substantial example…"

  • 4.4.4: In the paragraph after the second code block, the last sentence should read, "Were a user was able to change the constructor resolution…" It currently reads "where", even though that's not a possibility in the current code.

  • 4.4.4: Toward the end of this section, there is a reference to Listing 4.14. It should be referencing Listing 4.15.

  • 4.6: In the 13th bullet, it should read, "An constructor that is used for Constructor Injection should do no more than applying Guard Clauses and store the receiving dependencies."

  • 4.6: In the 18th bullet, it should be e.g., "This can be useful for add-in scenarios where some runtime context needs to be passed along to the add-in's public API…"

  • 4.6: In the 20th bullet, "either" should be removed, as it's referring to a choice between three options.

  • 4.6: In the 21st bullet, it should read, "A Local Default is a default implementation of a Dependency that originates in the same module or layer."

  • 4.6: In the second-to-last bullet, it should read, "Constructor Injection simplifies the class, and allows the Composition root to be in control over the value that all consumers get, and it prevents Temporal Coupling."

  • Punctuation and Formatting
    The items in this section aren't necessarily typos—or even incorrect, per se—but are minor changes, primarily to the punctuation and formatting, that might improve the flow by e.g. joining related ideas, separating out concepts, or emphasizing an identifier as code.

  • 4.1.3: In the annotations for Listing 4.3 and Listing 4.4, "HttpContext" (4.2), "IControllerActivator" (4.2), and "HomeController" (4.3) should be monospaced.

  • 4.2.2: In the Warning, there is an extraneous comma: "A Local Default with Dependencies, will become a Foreign Default when one of its Dependencies is a Foreign Default."

  • 4.2.2: After the Tip there is an extra comma at: "These are good examples of Dependencies where the local library can supply no good default implementation, because the proper implementations belong in specialized Data Access libraries."

  • 4.2.2: In Table 4.2 there is an extra comma: "Frameworks that apply the Constrained Construction anti-pattern, can make using Constructor Injection difficult."

  • 4.3.2: In the second paragraph, "The Dependency can vary with each method call, when the Dependency itself represents a value…"

  • 4.3.2: In the paragraph after Listing 4.10 consider breaking apart the note about context with em-dashes, as in, "Each time the Apply method is invoked on an add-in, the operation's contextrepresented by the context fieldis passed as a method parameter:"

  • 4.3.2: "Entities that contain behaviors besides their obvious set of data members, would easily get a wide range of methods…"

  • 4.3.2: "This makes testing the logic of an entity much harder, since all Dependencies need to be supplied to the constructor…"

  • 4.3.2: In the Temporal Coupling Code Smell callout, "Initialize" should either be monospaced (to imply that it's a literal method name) or lowercase (to imply that it's a conceptual method reference).

  • 4.3.2: In the Temporal Coupling Code Smell callout, "`EndpointAddress `" should be "`EndPointAddress` ". I assume this was meant as markdown to monospace it.

  • 4.3.3: After the second code block, "context" should be monospace as it refers to a parameter name.

  • 4.6: In the first bullet point, it should be a colon, not a semicolon: "The construction of your application's components should be concentrated into a single area of your application: the Composition Root."

  • Readability Suggestions
    The following are areas I found myself tripping over the wording, possibly due to repetition of words, run-on sentences, or simply words that, while correct, interfered with the readability of the content. These are fairly subjective, and more subject to stylistic preferences, so take them with a grain of salt. At the same time, these are also the trickiest issues for me to pick up when proof-reading my own writing, so I figured I'd document them.

  • 4: In the paragraph before the bullet list, consider "The main purpose of a design pattern is simply to provide a detailed and self-contained description…" since the purpose, in context, is to emphasize that design patterns aren't anything to be afraid of.

  • 4: In the paragraph before the bullet list, consider "And besides, you already saw some examples of three out of the four of the Design Patterns…"; it's the same message, but flows a bit easier.

  • 4.1: In the paragraph after Figure 4.1, consider "That consumer, however, also pushed the responsibility for creating its dependencies up as well." It's the same message, but less wordy.

  • 4.1: Right before Listing 4.2, consider "If we were to have a console application that was written to operate on this particular object graph, this might look like the following listing." Fewer words, easier to read.

  • 4.1.1: Should this section be called, "How the Composition Root works"?

  • 4.1.1: In the bullet list after the first Note, consider "A console application is an executable (.exe) containing a Program class with a Main method" to avoid repetition of "with… with…"

  • 4.1.1: In the bullet list after the first Note, consider "An SP.NET Core web application is a library (.dll), also containing a Main method as well." At minimum, it should be "a Main method", and, if retained, "it" shouldn't be capitalized.

  • 4.1.1: Prior to the second note, consider changing "both holds", "Even though the assembly that holds both the Composition Root and the User Interface Layer depends on…"

  • 4.1.1: Consider minor rewording as, "With ASP.NET Core MVC, for instance, although it is trivial to move Controllers and View Models to a separate project, but it can be quite challenging to do the same with your views and client resources." Otherwise, the "…for instance, although…" can be a bit of a mouthful.

  • 4.1.1: Consider, "Separating the presentation technology from the Composition Root might not be that beneficial, either, since a Composition Root is specific to that specific application." The first change emphasizes that this is extending on the previous concern, while the latter avoids the unnecessary repetition of "specific".

  • 4.1.1: In the final Note of the section, it might read better as, "Moving the composition of classes out of the Composition Root…"

  • 4.1.1: In the final Warning, consider "This also means that…" instead of the wordier "This does mean though that…"

  • 4.1.2: Just because "great" tends to be a bit of a cliche, consider e.g., "This also has the significant benefit of removing any coupling…"

  • 4.1.2: In the third paragraph, consider italicizing "Only" to emphasize the exclusivity being promoted here. Otherwise, it's easy to miss the distinction from the previous paragraph.

  • 4.1.3: After the first Note, two paragraphs begin with "Because…". To avoid this, consider changing the first to e.g., "Because The Startup.ConfigurationServices method only runs once; as a result, your CommerceControllerActivator class is…"

  • 4.1.3: After Figure 4.2, consider replacing "All the rest of the…" with simply "The remaining…"

  • 4.1.4: Since it's already established that we're talking about a subset of developers, replace "To get a good view of what it is some developers are worried about…", try simply, "To get a good view of what they are worried about…"

  • 4.1.4: Minor modification: "The above diagram however is deceiving, however, since assembly dependencies are transitive." This ensures the "however" is set apart by commas while placing it at a more comfortable transition point in the sentence.

  • 4.1.4: Consider e.g., "Since in Mary's application the User Interface library depends on the Domain library, and the Domain library depends on the Data Access library, this implies that the User Interface library also depends on the Data Access library."

  • 4.1.4: In the second-to-last paragraph, change to, "A Composition Root is the result of pushing the responsibility for creating dependencies up to consumers." This is less wordy due to the elimination of two "of"s and one "the" (i.e., "…the result of pushing the responsibility of the creation of dependencies…")

  • 4.2.1: Before the Tip, you could probably just remove "…at this point". If you keep it, remove the comma after it.

  • 4.2.2: Rephrase as, "Some frameworks, and especially older ones, assume that your classes will have a default constructor. This is called the Constrained Construction anti-pattern; we'll discuss this anti-pattern in more detail in the next chapter."

  • 4.2.2: Consider rephrasing as, "In this case, the framework will need special help creating instances when a default constructor is not available." This is a bit more explicit.

  • 4.2.2: Consider rewording as, "In extremely rare cases this may be a real issue, but in chapter 8, we'll describe how to delay the creation of a Dependency as one possible remedy to this issue. For now, we'll merely observe that there may (in fringe cases) be a potential issue with initial load and move on.

  • 4.2.2: In the Extremely big object graphs callout, consider rephrasing to e.g., "I (Steven) once had a conversation with a developer who had switched DI Containers. Having had some severe performance problems with Ninject, he had moved from Ninject to Simple Injector. Afterward, he reported a 300 to 400 millsecond speed-up per request, after switching to Simple Injector, which is obviously quite impressive." This flows better and is less repetitive.

  • 4.2.2: In the Extremely big object graphs callout, consider rewording as e.g., "The size of this object graph with a size that was unimaginable to me. I had never seen anything so hideously big before.”

  • 4.2.2: In the Extremely big object graphs callout, consider "…the moral of the story is actually that…" and "But, in the end, the number of objects in the graphs of well-designed systems…"

  • 4.2.2: In the Extremely big object graphs callout, consider ending with "…which reduces the number of objects to create per request even further" to remove the redundant "make".

  • 4.2.3: In the second paragraph, the last sentence is pretty lengthy and includes two "but"s. Consider rewording as e.g., "…based on the specified path. Here are the StreamWriter constructors; The StreamWriter constructors are similar:"

  • 4.2.3: To avoid repetition of "class", consider e.g., "Stream is an abstract class…" instead of "The Stream class is an abstract class…"

  • 4.2.3: To avoid repetition of "can" consider e.g., "Although the BCL provides us with examples where we can see Constructor Injection in use…"

  • 4.2.4: To reduce wordiness consider e.g., "Can she write some new code that enables the app to display prices in different currencies and calculate total costs in different currencies too?" This is taking a small liberty in treating "price" and "total costs" as synonymous, but I think it gets the point across.

  • 4.3.2: Prior to Listing 4.11, consider clarifying with e.g., "Constructor Injection leads to a situation where each such entity needs to be created with all of its Dependencies, even though only a few may be used for a given use case."

  • 4.3.2: In Listing 4.11's annotation #2, minor adjustments: "Both of the entity's methods, RedeemVoucher and MakePreferred, accept the required Dependencies using Method Injection."

  • 4.3.4: In Listing 4.15, annotation #1, it should read, "The ICurrencyConverter is now supplied through Method Injection." The current "now is" is not incorrect, but is less common (and, therefore, is a bit disruptive).

  • 4.3.4: Consider, after Listing 4.15, "…she prevented having to build up the Product entity with all of its Dependencies."

  • 4.3.4: The following would flow a bit easier, "…because it's not that important from both the point of view of either Method Injection or Constructor Injection."

  • 4.4.2: This is a bit subtle, but I'd replace, "In case the Dependency is required…" with simply, "If the Dependency is required…" The two are synonymous, but the latter reads as more deterministic (which is valuable here.) "In case…" can have a connotation of referring to an exception, even though required dependencies are really the rule.

  • 4.4.2: In the last sentence of the third paragraph, consider "This is often done by introducing Abstractions within a single module and letting classes within that module communicate via Abstractions…" This eliminates the contextually implicit "single module" while further clarifying that we're talking about internal communication.

  • 4.4.2: In the fourth paragraph, consider "The main purpose of applying loose coupling within a module boundary is to opening classes for extensibility." This eliminates the duplicate "for" and reads slightly better.

  • 4.4.2: In the Open/Closed Principle callout, consider removing the "but" in: "The Open/Closed Principle pushes us to a design where every new feature request can be done by creating one or more new classes or modules, but without touching any of the existing ones."

  • 4.4.2: In the Open/Closed Principle callout, the last line would read easier as, "Working with Abstractions is, of course, one of the main topics in this book, and there's more to it; we will explore some techniques that can help…" Otherwise, the "but" is a bit misleading as it's not really contrasting the previous point, but emphasizing it.

  • 4.4.2: After the Reusable Library callout, I'd add, "Appearances can be deceptive, though and Property Injection is fraught with difficulties." This emphasizes that this is a counterpoint to the previous paragraph.

  • 4.4.2: In the second-to-last paragraph, it should be "we wire up our classes in our Composition Root." (I suppose "wire our classes" works, too, but "wire up" is more common.)

  • 4.4.3: To emphasize that it's an example, add, "For example, System.ComponentModel.IComponent has a writable Site property that allows you to define an ISite instance."

  • 4.4.4: Consider clarifying as, "…this would be misleading, since Property Injection is hardly ever a good fit when building applications; Constructor Injection is almost always a better choice." (I also made some minor punctuation and wording adjustments.)

  • 4.4.4: I would change it so it reads, "…so we won't go into much detail about it here." I don't believe this is strictly required, but it avoids confusion with (the otherwise synonymous, in this case) "…so we won't go in too much detail about it here."

  • 4.4.4: I would also change, "One of these behaviors that Simple Injector allows replacing…" This is another case where both work, but this reads a bit better.

  • 4.4.4: Change to, "It is used to specify the relationships between Abstractions and implementations, and it is used to build object graphs of these implementations." Both work, but "is used to" is implicit here, and removing it is less wordy.

  • 4.4.4: Change to, "The class includes an Options property of type ContainerOptions. The ContainerOptions includes a number of properties and methods…". The use of "contains" is just fine, but since we're talking about "Containers" it makes the sentences read better.

  • 4.4.4: In the paragraph after the second code block, merge the first two sentences: "The property can be changed multiple timeswhich is something that can be very convenient for the useras long as there are no registrations made in the container." This avoids the repetition of "The property can be changed".

  • 4.6: In the eleventh bullet, consider "This means that application code can't pass on Dependencies…" instead of the wordier "This does mean though that…"

  • 4.6: In the 15th bullet, I believe it should be, "Constructor Injection is well-suited for when the Dependency is required."

  • 4.6: In the 19th bullet, consider simply, "Method Injection is unsuited for use inside the Composition Root because it leads to Temporal Coupling."

  • Content Suggestions
    The following are more invasive changes related to organization and order of content, technical details, &c.

  • 4: In the second-to-last paragraph, the sentence, "However, Composition Root and Constructor Injection are by far the most important of the four patterns" is largely redundant with the final sentence in the section, "The most important patterns are Composition Root and Constructor Injection, which you should use in most situations…" I'd simply remove the first sentence, as it's a bit extraneous to that paragraph.

  • 4.1: I'd extend the Definition to, "A Composition Root is a (preferably) unique location in an application where modules are composed together into one or more object graphs accounting for all dependencies required by the application or request." This is all discussed in the subsequent text, but it'd be useful to capture it in the Definition for people referencing the chapter.

  • 4.1: Consider adding to the final paragraph e.g., "Separating it into its own method, however, can help ensure that the composition is consolidated, and not otherwise interspersed with subsequent application logic."

  • 4.1.1: At the end of the second paragraph, consider adding something like, "In addition, for reusable libraries distributed among multiple applications, this ensures that dependencies are managed in an area outside of the core library code, where implementors have full access, while also accounting for environment- or client-specific requirements."

  • 4.1.2: In the third paragraph, consider adding e.g., "The rest of the application has no reference to the container—not even through an abstraction—and instead relies on the patterns described in this chapter." Otherwise, it's easy for a naive reading to conclude that the DI Container just needs to be treated as a Volatile Dependency, which is obviously not correct.

  • 4.1.2: Consider adding, "You don't have to worry about that because it's almost never the case, and, in the very few situations where it is, there are ways to address the issue, as we'll discuss in Chapter 8.4.2." That way, you're not just leaving the implied question unanswered, but cross-referencing the solution. (Be aware that I haven't read Chapter 8.4.2 yet, so I could be wrong on the chapter reference.)

  • 4.1.3: In Listing 4.3, the CommerceControllerActivator constructor is called with two parameters: accessor and connectionString. In Listing 4.4, however, which defines the CommerceControllerActivator implementation, the constructor only accepts one parameter: connectionString. I assume this pertains to Listing 4.3's annotation #4, so you may already be monitoring this. Regardless, the code references should be synchronized.

  • 4.1.4: I was initially confused by why there were six dependencies in Figure 4.4. It may be worth clarifying in the explanation with something like, "Remember, though, that dependencies are not defined by the number of modules, but the number of times each module depends on another module. As a result, the total number of dependencies between all modules in Mary's application is, in fact, six."

  • 4.2.2: To acknowledge the previous discussion on this topic, consider adding e.g., "As previously discussed in Chapter 4.1 Composition Root, an apparent disadvantage of Constructor Injection is that…"

  • 4.2.4: Listing 4.8 implies that the IUserContextAdapter now has a Currency property, but this isn't discussed anywhere else in the section. I don't think that's strictly necessary, but it's worth acknowledged in either an annotation or the explanation of the listing.

  • 4.3.2: In the paragraph two before Listing 4.11, consider adding something like, "In addition, since entities are frequently cached, this may introduce lifetime mismatches with any dependencies injected via the constructor—and, especially, for any dependencies that are request- or user-specific."

  • 4.3.2: This is a bit of a broader issue, but in Listing 4.11, one might reasonably infer that the injection of the IVoucherRedemptionService creates an indirect dependency between the Customer entity and an unlisted IVoucherRepository Data Access service. This gets into an issue that you two have addressed on Stack Overflow previously, and which comes up a lot around Dependency Injection: injecting repositories into entities. This may be worth addressing—either here or, if you haven't already, later.

  • 4.3.4: The second-to-last paragraph was confusing to me, primarily because it assumes a relatively narrow perspective. The caller of the class is supplying it one way or the other: either via the constructor or via the method. And while the method may "have no choice in how to model DI or whether [it needs] the Dependency at all", that is really only true in the case of plug-ins (i.e., "the Dependency's consumer can vary on each call"); in other cases, we're presumably requesting the dependency in the method because we know that we need it, but believe it's better suited for the method (i.e., because "the Dependency can vary on each call"). Consider if there's a way to clarify this.

  • 4.4: The bulk of this section reads like an anti-pattern. I understand why that is, since Property Injection solves a really specific problem, and outside of that it is frequently abused. Since this is in Chapter 4 DI Patterns, however, I'd recommend making the positive case for solving that specific problem. In addition, you might also consider adding a section called e.g., When not to use Property Injection, which would consolidate all of the potential problems and abuses associated with it.

  • 4.4.1: Related to the previous feedback, 4.4.1 How Property Injection works takes a considerably different approach than previous chapters on Dependency Injection techniques. Instead of providing reasonable code showing how to implement it, it sets up examples of how not to do it. Contrast this to e.g. 4.4.1 How Constructor Injection works, for example, which provides a Guard Clause in the first listing, and then explains why this is necessary. Since the pattern is defined as letting "callers supply a Dependency if they wish to override the default behavior", it would be reasonable, at minimum, to provide a Guard Clause and a Local Default. (Acknowledging that the Temporal Coupling issue is more involved, and may not be easy to implement here.)

  • 4.4.2: Another area I frequently see Property Injection used is as an attempt to handle optional cross-cutting concerns, e.g. by allowing an optional logging class to be injected. Now, obviously, there are much smarter (and less invasive) ways of handling this, as you'll discuss later, but it may be worth calling out as an(other) (ab)use of Property Injection. (Assuming, of course, that you two have encountered similar code smells.)

  • 4.4.2: In the Open/Closed Principle callout, the second sentence of the first bullet isn't true when discussing reusable libraries (as discussed shortly after). It also may not be true when a project is distributed across multiple teams. You may want to clarify this with e.g., "This statement, by itself, is a bit dull assuming our team owns the entire application, since we can always change any part of the system."

  • 4.4.2: I'd remove "…or mistakenly supply null as a value" as this is no less of a case with the generally-preferred Constructor Injection, nor is it any more difficult to mitigate via a Guard Clause.

  • 4.4.2: In the second-to-last paragraph, when discussing the benefits of the Composition Root, it may be worth adding e.g., "This also makes it clear what the extensibility points are by effectively documenting them in a single location."

  • 4.4.3: In the first sentence, consider expanding on the idea, "In the .NET BCL, Property Injection is a bit more common than Constructor Injection—probably because good Local Defaults are defined in many places, and also because this simplifies the default instantiation of most classes."

  • 4.4.4: In annotation #3 of the second code block, add a reference to footnote #45. This is previously cited, but as that reference didn't specifically use the term "Popsicle immutability", this may be an unfamiliar term to the reader (unless they were previously aware of it, or happened to flip forward to check the citation while reading).

  • 4.4.4: After the second code block, you note "The property can be changed multiple times, which is something that can be very convenient for the user." Can you include a scenario where this would be true? The only scenario I can think of would be if we were passing a single instance across multiple classes or methods for configuration—but, generally, we wouldn't be doing that, and especially since all dependencies injected via Property Injection are to be defined in the Composition Root. Are there scenarios I'm failing to consider?

  • 4.4.4: At the end of the paragraph following the second code block, consider adding, "…it could impact the correctness of earlier registrations, and especially in terms of managing the lifetime of dependencies." I'm assuming here that this would cause issues because e.g. there might be a conflict with a previously-created singletons.

  • 4.4.4: Consider clarifying, "…and, for dependencies with lifetimes persisting beyond the current request, we even have to consider thread safety". Since "Property Injection is, just as Constructor Injection, applied from within the Composition Root", I'm assuming thread safety would only be an issue with persistent objects that might be updated from multiple requests simultaneously.

  • 4.5: I may be over-thinking this, but, in the bullet list, you say, "In case we're mixing runtime data with behavior in the same class, as we might do in our Domain entities, Method Injection is a good fit." That's certainly true of Domain entities, as stated, but might that condition also be inadvertently true of Domain libraries, such as the IProductService? Or is the assumption that the Domain library would only include behavior, with any "runtime data" being fed from the Data Access library and, thus, handled by Domain entities? I ask because I'd consider e.g. the IUserContextAdapter to contain "runtime data" too, and while it's originating in the Composition Root, and ultimately being relayed to a Domain entity, it's certainly something the Domain library itself has to be aware of. And, yet, I wouldn’t expect it to be injected into the IUserContextAdapter via Method Injection.

  • General Feedback
    There's no action necessary on the following; they're just comments that popped up as I was going through the book.

  • 4: "A sauce bearnaise is really a sauce hollandaise…"—that's fascinating, I had no idea!

  • 4: This isn't specific to the Second Edition, but I continue to love that you've defined concrete rules for when each injection pattern makes sense. Prior to reading this book the first time, it seemed like developers were choosing these techniques based on stylistic preference. And, perhaps, that remains true for some developers today. But you make a solid argument for why each is better suited for specific scenarios. (Of some irony here, when I first started exploring DI a number of years ago, the first post I came across recommended using Property Injection for everything (!!). Fortunately, I kept digging into the question, and would eventually discover Mark's posts on Stack Overflow.)

  • 4.1: The two paragraphs after Figure 4.1 are a really nice introduction to the Composition Root. In fact, if it weren't for the cadence of the question/answer opening to each pattern, they might have even made a better introduction than the current one. Regardless, they really get to the heart of Constructor Injection.

  • 4.1.1: I appreciate the paragraph starting with, "Don't be misled to think that the Composition Root is part of your User Interface Layer." This is a useful distinction, and a conceptual point that I've previously had trouble articulating.

  • 4.2: Figure 4.5 is a useful diagram. I appreciate the use of annotations to articulate in plain English what each piece is doing, and what it's called. It helps reinforce a vocabulary around dependency injection.

  • 4.2.2+: In e.g. Table 4.1, the tabular format doesn't really complement the content very well. As a table, the advantages on the left don't correspond directly to the disadvantages on the right, and the columnar format forces some of each to wrap. For this, and similar subsequent tables, I'd consider just listing these out as bullets instead of as a table—or, at least, putting the Disadvantages in a separate row instead of as a column.

  • 4.4.2: As someone that primarily works on reusable libraries, I've taken issue with a few points in the book that assume that developers of one library will have access to code in other libraries. Given that, I really appreciate the Reusable Library callout. (Though I've also suggested clarifications in previous sections when this assumption might be inappropriate.) As a general rule, I'd argue that treating each module as though it's a reusable library, even if it isn't intended to be, is a good way of thinking through Dependency Injection issues.

  • 4.4.4: "To explain Property Injection, this example uses the Constructor Injection feature of a DI Container. That's like watching the movie Inception with Leonardo DiCaprio."—Ha!

  • 4.5: Figure 4.8 is really useful.

  • Jeremy
    Jeremy Caney (28) [Avatar] Offline
    Hyphen, Comma Conventions

    I removed the following from the Punctuation and Formatting section because a) they were a bit nit-picky, and b) you two used your conventions so consistently that it made me second-guess whether my own usage was a matter of correctness or simply a stylistic preference. Nevertheless, I'm placing them here in case this is something you (and/or your copy editor) want to change—otherwise, feel free to disregard this comment entirely!
    Note: In some of these cases, the consistent usage can also be harder to read—e.g., when you have "however" and "for instance" back-to-back ("…, however, for instance, …") this can interrupt the flow. In other areas, however, not having any commas can also make it a mouthful ("…however for instance…").

  • 4.1: "When composing an application from many loosely-coupled classes…"

  • 4.1: "We can't, of course, delay the creation of our classes indefinitely."

  • 4.1.1: "from taking a dependency on, for instance, the Data Access Layer project."

  • 4.1.4: "In their old, tightly-coupled code bases"

  • 4.1.4: "It's not hard to imagine how the number of dependencies in a tightly-coupled code base actually explodes compared to a loosely-coupled code base."

  • 4.2.2: "After doing some analysis on his application, though, I found out that, in some cases, an object graph was created that consisted of…"

  • 4.2.3: "For our application components, however, it doesn't."

  • 4.3.2: In the Temporal Coupling Code Smell, "Semantically, the name of the Initialize is obviously a clue…"

  • 4.3.3: "the component may carry information about which IDesigner to create, but, at the same time, it's also the component upon which the designer must subsequently operate."

  • 4.3.3: After the second code block, "The IValidationAttributeAdapterProvider interface, for instance, can be used for…"

  • 4.4.4: "Because of this, Simple Injector, by default, only allows classes to be created in case there is only one public constructor."

  • 4.4.4: "Simple Injector, however, allows you to override this behavior."

  • 4.4.4: "It may look simple in its raw form as shown in listing 4.15, but, properly implemented, it tends to be much more complex…"

  • 4.5: "Figure 4.8 shows a decision process that can help you decide on a proper pattern, but, if in doubt, choose Constructor Injection…"

  • 4.6: In the 15th bullet, "It is important to note, however, that Dependencies should hardly ever be optional."

  • Jeremy Caney (28) [Avatar] Offline

    Aside, I see that the "Welcome" post explicitly states:
    During the MEAP, we welcome reader comments about anything in the manuscript — other than typos and other simple mistakes. These will be cleaned up during production of the book by copyeditors and proofreaders. It is not as helpful for the author to focus on these types of errors at an early stage of writing, while feedback on the content is most helpful.

    I'm going to make a sincere effort to focus on Content Suggestions and General Feedback for future chapters. Are the Readability Suggestions also useful? Or are those best left for the copy editors?

    Steven van Deursen (25) [Avatar] Offline
    You are right, the welcome note explicitly states that typos and other simple mistakes don't need to be reported. They will eventually be fixed before the final version.

    My experience thus far is that most readers that report typos simply report a single typo here on the forum. Although I gratefully thank them, this isn't that useful for me, since there is a certain amount of overhead in fixing a single mistake.

    You, on the other hand, are reporting a vast number of typos and simple mistakes that for the most part haven't been fixed. I personally find this feedback really useful, because having such big list allows me to quickly fix them without much overhead. Having these fixes now means that they will be included in the next MEAP release. This improves the readability for the MEAP readers, which is something I am focusing on, since I consider these chapters for the most part ‘done’ (from perspective of the content that is). In other words, I really appreciate this big list of suggestions, so feel free to continue them.

    I can imagine reporting these errors by itself takes a lot of time, so when time becomes an issue to you, please consider skipping reporting typos and simple mistakes. Especially compared to the readability suggestions, content suggestions and general feedback, since those are extremely valuable to us. So whatever you do, please continue giving us your readability suggestions, content suggestions and general feedback.

    Thanks again
    Steven van Deursen (25) [Avatar] Offline
    But to prevent 'polluting' this forum, we might want to consider a different medium. Want to mail your suggestions for the next chapters?
    Jeremy Caney (28) [Avatar] Offline

    That makes good sense about the piece-meal updates. I'm also glad to hear, as it takes a certain amount of discipline for me not to report typos! (Once upon a time I was a copy editor, and even now my job largely consists of reviewing technical documents and code.)

    I'll reach out to you to figure out the best medium and format for feedback.

    Steven van Deursen (25) [Avatar] Offline
    4.4.2: Another area I frequently see Property Injection used is as an attempt to handle optional cross-cutting concerns, e.g. by allowing an optional logging class to be injected. Now, obviously, there are much smarter (and less invasive) ways of handling this, as you'll discuss later, but it may be worth calling out as an(other) (ab)use of Property Injection. (Assuming, of course, that you two have encountered similar code smells.)

    I think I can easily speak for Mark as well when I say we have encountered similar code smells on a regular basis. A very similar subject is discussed in section 5.3 about Ambient Context. Although it doesn't discuss Property Injection, the underlying problems are identical and this section even talks about logging as example.

    We'll consider adding a note and forward reference to 5.3

    4.4.4: After the second code block, you note "The property can be changed multiple times, which is something that can be very convenient for the user." Can you include a scenario where this would be true? The only scenario I can think of would be if we were passing a single instance across multiple classes or methods for configuration—but, generally, we wouldn't be doing that, and especially since all dependencies injected via Property Injection are to be defined in the Composition Root. Are there scenarios I'm failing to consider?

    I decided to remove the wording "which can be very convenient for the user", because it isn't that relevent and only raises questions (like yours). But to answer your question, it's not unusual to have extension methods that extend the original behavior by replacing the original IConstructorResolutionBehavior with a new implementation that wraps the original behavior (a decorator). For instance:

    public void ContainerOptions PagesCanHaveMultipleConstructor(
        this ContainerOptions options)
        options.ConstructorResolutionBehavior =
            new PagesCanHaveMultipleConstructorBehavior(
                original: options.ConstructorResolutionBehavior);    

    When the user uses multiple of these extension methods, his Composition Root can become the following:

    var convention = container.Options.RegisterConstructorSelectorConvention();

    These extension methods would each replace the original configured ConstructorResolutionBehavior, which means the property would be changed 3 times. Only allowing the behavior to be replaced once would disallow this common API approach.