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

First off, let me say: I'm really happy that you're working on this second edition! I've been a big advocate of the first edition, but often find developers dismissive of picking up a six-year-old technology book—even though most of the lessons are as relevant today as they were when Mark first penned them. In addition, this is a welcome refresh, and I'm happy to see it updated both for newer frameworks but also evolving patterns.
Note: While I did end up with a lot of feedback, this looks even longer than it is because of the overhead of establishing a precise location (e.g., by quoting contextual content).

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

  • 1.1.1: In the "Abstract Factory" callout, there's an extraneous "you" in "…but you depending on the application or toolkit, there could be many more"

  • 1.1.2: Remove the redundant and unnecessary "illustrates" in "This is illustrated in figure 1.6 illustrates."

  • 1.1.2: Remove unnecessary comma after "…without touching existing parts of the system, means that our problems get isolated."

  • 1.2.2: In the Intercepting text messages callout, it should read, "During manual testing, on the other hand, text messages were actually sent…" It currently reads, incorrectly, "where" (also note the missing comma).

  • 1.3.2: In the second bullet point, it should be an "or" not a "but": "The Dependency doesn't yet exist, or is still in development".

  • 1.4: In the caption for Figure 1.12, it should read "The main method takes responsibility for the creation…" not "of the".

  • 1.4.1: In the first paragraph, "we se fit" should be "we see fit".

  • 1.6: "Miscreant" should be plural; "…because the scope of likely miscreants decreases."

  • 1.6: "A DI Container is useful, but optional tool" should be "A DI Container is a useful, but optional tool".


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

  • Welcome: "You should be able to design complete loosely coupled applications" would read easier as "You should be able to design complete, loosely-coupled applications".

  • 1.1.1: In the final Note toward the end of the section, consider either "…or how and when…" or "and how—and when".

  • 1.1.2: In the sentence where you introduce the concept of a Null Object, consider a semi-colon to help break up the concepts: "…it corresponds to having a children's safety outlet plug; i.e., a plug without a wire or appliance…" (this is a good analogy, by the way).

  • 1.1.2: Consider "with no changes to other already-existing classes of the system" (hyphenation).

  • 1.2.1: Under Collaborators, "the Salutation class" should be "the Salutation class" (not monospace).

  • 1.2.1: Under Collaborators, after Listing 1.1, the reference to the Write method should be monospace ("…you only need the Write method.")

  • 1.2.2: It might be appropriate to hyphenate "overly-simplistic" in the first paragraph since it's a compound concept.

  • 1.2.2: In the second-to-last paragraph in the Late Binding section, IMessageWriter and Salutation should be monospaced.

  • 1.2.2: The callout box Intercepting text messages. has an unnecessary and inconsistent period at the end.

  • 1.3.2: In the second paragraph of the first bullet point, "Volatile" should be lowercase ("…it isn't so much the concrete .NET types that are volatile, but…".

  • 1.5: "loosely coupled" should be hyphenated since it's being used as an adjective; "…whereas a well-designed, loosely-coupled code base…".

  • 1.6: "loosely coupled" should be hyphenated since it's being as as an adjective; e.g., "….When we have a loosely-coupled infrastructure in place…'

  • 1.6: "So-called" should be hyphenated.


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

  • Welcome: The final paragraph repeats "hope" and "more"; consider revising to avoid repetition.

  • Part 1: The sentence, "Both chapters are almost shared like a narrative" is a bit awkward, and could probably be safely removed since the idea is implicit in the paragraph.

  • Part 1: In the second to last paragraph, consider renaming "containers" to "tools" to avoid further repetition the term "container" in a paragraph that already uses the term six times (this isn't entirely avoidable, but that last one is easy to replace).

  • 1: You can probably remove "…and are curious about it"; I think that's safe to assume since they are reading the book!

  • 1.1.1: You can safely remove "…about it" in the first paragraph; it isn't necessary and flows better without it.

  • 1.1.1: Consider "these misconceptions" instead of "the misconceptions" in the second paragraph; while "the misconceptions" is technically correct, "these" flows better with the reference to "those" later in the sentence.

  • 1.1.1: When discussing Abstract Factories, the first two sentences in the callout are somewhat redundant; I'd consider removing the first sentence ("The Abstract Factory pattern…"), and just introducing the concept with the second sentence ("An Abstract Factory is typically…").

  • 1.1.1: In the final Note toward the end of the section, replace the final "this" with "it"; "this" is already defined, so "it" is a bit clearer and less repetitive.

  • 1.1.1: At the end, the "people tend to try to" is wordy; it can be simplified to simply "people try to" or, if you want to soften it, "people often try to".

  • 1.1.1: The final sentence in the section ("Let's assume that you don't know anything…") doesn't add much to the previous sentence, and feels a bit tedious as a transition; I'd consider removing it.

  • 1.1.2: Under Checking into a cheap hotel, there are two references to "apparently" in the first paragraph; see if you can't remove one of them.

  • 1.1.2: Consider: "…the socket is an interface and the plug with its appliance is the implementation".

  • 1.1.2: Consider: "…either the client or the implementation" in the paragraph about the Liskov Substitution Principle.

  • 1.1.2: The applicability of the socket-and-plug pattern is described as "amazing" twice; consider replacing one of them with e.g. "remarkable".

  • 1.1.2: To reduce repetition, consider: "…a new requirement should only necessitate the addition of a new class…" (instead of "…only require the addition…").

  • 1.1.2: Consider removing "…we get closer…" as it's contextually inferred: "And with every step we get closer, it gets easier to add new features…"

  • 1.1.2: Consider "…means that any problems are isolated" instead of "…means that our problems get isolated" (since problems are not assured—albeit perhaps probable).

  • 1.1.2: To reduce repetition, replace "loose coupling" with "it" in "That's what loose coupling can help us with, and that's why it can make a code base more maintainable."

  • 1.2.1: Under Collaborators, consider expanding with e.g. "you can execute the logic via the Exclaim() method…" so it's clear what's executing the logic.

  • 1.2.2: Under Late Binding, you can merge the following two sentences without losing much, "However, you can introduce late binding by simply changing this single line of code".

  • 1.2.2: Just after the Testability callout, consider replacing "the benefits" with simply "those" since it's implicit that you're referring to benefits; this will remove the repetition of "the benefit(s)".

  • 1.2.2: Consider removing, "Although integration tests typically communicate with a real external systems (like a database), you'll still find the need to have a certain degree of isolation during integration testing" as it's contextually implicit.

  • 1.2.2: In the callout box Intercepting text messages just say, "Steven has worked on…"; since it's in a callout box, the "for instance" transition is a bit orphaned from context.

  • 1.2.2: In the callout box Intercepting text messages remove one of the instances of "certainly".

  • 1.3: In the second sentence, replace "good idea" with "benefit"; this avoids the repetition of "good idea" in the next sentence.

  • 1.3: To reduce wordiness, consider changing "…loose coupling provides especially useful guidance" to simply "…loose coupling proves especially useful".

  • 1.3: To differentiate from the preceding idea, consider e.g., "You don't have to abstract everything away and make it pluggable, however." This acknowledges that it's providing a counterpoint to the previous guidance. (I also replaced the redundant "everything" with "it" since that was inferred.)

  • 1.4: Replace "Next we'll cover them briefly…" with "Next, we'll cover each of these briefly…"; it flows better.

  • 1.4.1: "…when new requirements come in…" might read better as "…when new requirements are introduced…".

  • 1.4.1: Consider adding e.g., "In fact, initially, DI was synonymous with Object Composition…" to reinforce that this is emphasizing the previous point.

  • 1.4.1: in the callout box, Dependency Injection or Inversion of Control?, merge the following sentences: "As soon as Main is invoked, your code is in full control of program flow, lifetime, everything." This avoids repetition of "control" and feels less stilted.

  • 1.4.2: Replace "With Interception…" with "Both Interception and Aspect-Oriented programming…" to better acknowledge the previous sentence (which introduces both, and not just interception.)

  • 1.6: "Unpredicted" is probably fine, but "unanticipated" or "unforeseen" are more common choices and might flow better.

  • 1.6: The sixth bullet should either be "DI enables simplified development on the same code base in parallel" or "DI simplifies parallel development on the same code base".

  • 1.6: The "…of the code base" at the end is redundant, and unnecessary; it can be safely removed.

  • 1.6: Change "…in case they are…" to "…in the case that they are…".


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

  • Welcome: "…since this book is about .NET Core…"—Is the book really about .NET Core? Or is the update just taking .NET Core into account? I'd consider clarifying.

  • Part 1: The second sentence of the last paragraph ("It's aimed at readers who…") is largely redundant with the last sentence of the three paragraphs before ("The chapter assumes you have no prior knowledge of DI…"); I'd consider just removing the latter, since the idea is well-covered in the final paragraph shortly after.

  • 1.1.1: Ending each section with a variation on "If you thought that DI was only relevant for [x], this is something you need to unlearn" feels a bit redundant and tedious; I think these can safely be removed (though I acknowledge there's a certain cadence to the repetition).

  • 1.1.1: When discussing Unit Testing, I'd consider spelling out why dependency injection aids in testability. E.g., that it allows for injecting test doubles into classes as a means of limiting the scope of a test while still maintaining runtime compatibility with (and contractual obligations of) dependencies. That should be obvious to people familiar with testing, and is obviously discussed later in 1.2.2, but it's useful to tie the concepts together upfront.

  • 1.1.1: When discussing Unit Testing, you may also want to note that if you only approach dependency injection insofar as required to satisfy the needs of testability, you may easily find yourself creating fairly brittle software, and thus not achieving the full benefit of DI. For instance, it's easy for a naive approach to inadvertently introduce a foreign default and, thus, the Control Freak anti-pattern discussed in Chapter 5.1.1.

  • 1.1.1: When discussing Abstract Factories, it'd be useful to cite some concrete, familiar examples of service locators, such as the ASP.NET Provider Model—though, otherwise, the concept is effectively covered with the abuse of DI Containers in the next section.

  • 1.1.1: At the end, after "In a sense, this whole book is one big argument against these common misconceptions", consider adding "…so we'll certainly be returning to these topics later. For example, Chapter 5.2 discusses why the service locator is an anti-pattern." That helps solidify the point of this paragraph with a concrete example.

  • 1.2: In the paragraph right after Figure 1.4, consider adding e.g. "—potentially even a customer-supplied hairdryer" to emphasize that the replacement might be the same type of appliance (if the customer has a preference) or something entirely different, as expanded on in the following paragraphs.

  • 1.1.2: I'd merge the paragraph "We can unplug the computer…" with the first sentence of the following paragraph ("If we unplug the computer…"); as two separate statements, it risks overextending the metaphor. E.g., "If we unplug the computer from the wall, neither the wall outlet nor the computer breaks down; even though nothing is plugged in, the room doesn't explode. With software, however…".

  • 1.1.2: When introducing the Null Object pattern, consider prefacing the pattern name as "the Null Object" or "a Null Object“; it will help it flow more naturally as part of the sentence.

  • 1.1.2: To really reinforce the point, consider adding e.g.: "In a sense, this is what this entire book is about: it's the core question that dependency injection seeks to answer."

  • 1.2.1: Under Collaborators, after Listing 1.1, I'd explicitly note that the Salutation class has no references or awareness of the ConsoleWriter class; it interacts with it exclusively through the IMessageWriter interface; this is implied, and also explicitly noted later in the chapter, but it's such a crucial point that I think it's worth highlighting upfront.

  • 1.2.1: To really reinforce the purpose of the Adapter design pattern, consider e.g. "The ConsoleMessageWriter class implements IMessageWriter by wrapping the Base Class Library's Console class; this is necessary since Console doesn't explicitly implement the IMessageWriter class, nor should it since the interface is specific to our application."

  • 1.2.1: The last couple of paragraphs (starting with "You may be wondering about the benefit…") would really make sense as the introduction to the next section, 1.2.2 Benefits of DI, rather than a transitional conclusion to 1.2.1 Hello DI code.

  • 1.2.2: The paragraph starting with "In table 1.1, we listed the late binding benefit first…" should be listed after the table it references; in my e-reader, at least, it shows up before.

  • 1.2.2: In the Late Binding section, the discussion about configuration may be better framed around an introduction to the idea of a Composition Root; without this, it may well invite the question "why not simply call the configuration from the Salutation class directly?"—which is obviously the wrong lesson to draw here. In this case, the configuration isn't any different from a service locator, which is only excusable since it's in the composition root. Likewise, since Main() is the composition root, you could alternatively opt for a code-over-configuration approach (as you discuss later). Regardless, by framing it around an early introduction to the Composition Root, it would help distinguish why setting dependencies in the Main() method (either directly, or indirectly via a configuration file) is conceptually distinct than the e.g. Salutation class (in this example).

  • 1.2.2: Toward the end of the Extensibility section. consider adding e.g. "…the Salutation class is unmodified because it only consumes the IMessageWriter interface. Similarly, there's no need to either modify or duplicate the functionality in the ConsoleWriter class, either.

  • 1.2.2: Under the Testability callout box, consider adding e.g. "Such manual tests are time consuming, expensive to perform, and susceptible to inconsistent coverage between tests…"

  • 1.2.2: In the paragraph before the Intercepting text messages callout, consider adding e.g. "Further, even without test doubles, the architecture implied by loose coupling can make it easier to isolate problems"; this benefit is stated elsewhere, but it's a useful point to reiterate.

  • 1.2.2: Before the Test Doubles callout, consider adding e.g., "Regardless, dependency injection provides options in the future with minimal additional overhead today." to emphasize that even if they find neither benefit especially compelling (!!) then they're still gaining flexibility by organizing their code this way upfront.

  • 1.2.2: In the callout box Test Doubles consider adding e.g., "These are useful when the real dependency is slow, expensive, destructive, or simply outside the scope of the current test" to help communicate why someone would want "stand-ins for the real or intended implementations".

  • 1.2.2: After Listing 1.3 you may want to note something along the lines of, "In this case, the test double is as involved as the actual production implementation. This is an artifact of how simple our example is, however. In most applications, a test double is significantly simpler than the concrete, production implementations it stands in for."

  • 1.2.2: In the last paragraph, consider adding, "The only major obstacle is to figure out how (and where) to get hold of instance of those interfaces" to give a hint as to the upcoming discussion of the Composition Root.

  • 1.2.2: In the last paragraph, consider adding, "Constructor Injection is the preferred method of doing that, though we'll also explore a few additional options in Chapter 4."

  • 1.3: This section dances around another myth that might be worth calling out, either here, or in the myths section: "Using dependency injection means never using the new keyword in an application". This is obviously false. The new keyword may ask the question of whether DI is necessary, but it doesn't answer it; Chapter 1.3 provides the yardstick necessary to address that.

  • 1.3.1: Can DI containers really be considered a Stable Dependency? It strikes me that someone using a DI Container may well want to swap it out with another DI Container at some point, or even convert it to a Pure DI approach. Indeed, you note that this is "yet another reason why you should limit the use of the container to the application's Composition Root—which only seems to reinforce that it is, in fact, a Volatile Dependency.

  • 1.3.2: The last sentence seems more appropriate as the first sentence of the next section, 1.4 Scope.

  • 1.4: Consider adding e.g., "…our Salutation class was released of the responsibility of creating its Dependency—i.e., ConsoleWriter, or another concrete implementation of the IMessageWriter interface"; this helps reinforce the relationship between the interface and the dependency.

  • 1.4: I'd move the first Note after the paragraph that starts with, "At first, it may seem like a disadvantage…" as it's that paragraph that introduces the concept of control, which the note then expands upon.

  • 1.4: In the first Note, I'd write, "As developers, we actually gain control…", in order to emphasize the apparent contraction with subsequent paragraph.

  • 1.4: I would add, "For instance, at first it may seem like a disadvantage to let a class surrender control…" to emphasize the fact that this is diving into a point introduced in the previous paragraph (i.e., that "as developers, we gain some advantages").

  • 1.4: When you say, "Many times, this is of no concern to the consumer, but in other cases it may be", the thought feels incomplete. It would be useful to either provide a concrete example, or at least add something like, "as we'll discuss shortly, in Section 1.4.3."

  • 1.4: To really hammer home the point of interception, I'd add e.g., "With DI, we gain the ability to intercept each Dependency instance and act upon it before it's passed to the consumer. This provides us with extensibility." (Note: I also changed "Dependencies" to be singular.)

  • 1.4.1: Merge the sentence "Although the original meaning of DI…" with the paragraph two before it, as they are largely redundant. E.g., add "Despite this, other aspects have also turned out to be relevant" to the paragraph starting with "Such Object Composition is often the foremost motivation for introducing DI into an application."

  • 1.4.1: In the callout box, Dependency Injection or Inversion of Control?, you reference WCF. Given that this has, for better or for worse, been largely displaced by ASP.NET Web API and, now, ASP.NET Core, it may be worth removing this reference from the second edition. The callout box works just fine without it.

  • 1.4.2: Near the end, consider adding "…things become much more complicated, as we'll discuss in Chapter 8.2." This is a bit repetitive with the next paragraph, which also references Chapter 8, but in doing so it reinforces that we'll return to the question of IDisposable explicitly, instead of leaving it without a solution (or even a full explanation).

  • 1.4.2: The last paragraph ("Giving up control of…") doesn't really add much value to what's already been stated; it can safely be removed. (It's useful to provide a conclusion in summary of a large section, but this section was just over a page.)

  • 1.4.3: In the first paragraph, "provide" might be a better word than "gain" since we're writing from the perspective of the class developer—we always had that control, but we're delegating that control to the third-party just mentioned.

  • 1.5: Consider adding to the final Note e.g., "This will become especially clear as we work through some more complex examples with nested dependencies in Chapter 4".


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

  • General: I really like having the callout style for definitions, information, and warnings, each with their own iconography. It really helps set these apart, while establishing a visual language for parsing the content.

  • General: I also really appreciate the visual annotations within the code; this looks really slick, and makes it much easier to cross-reference the code with the explanation.

  • Part 1: The introduction does a really good job of establishing the goal of the book, who it's for, and the concepts it describes.

  • 1: I appreciate you spelling out the ELI5 metaphor in the paragraph after the Stack Overflow quote, as that really brings it home.

  • 1.1.1: Under Abstract Factory, the paragraph starting with "What were your initial thoughts about this sentence?" is excellent. It's easy in the abstract language of software patterns to believe we have an understanding of a topic, while entirely missing the point.

  • 1.1.2: The clipart in this section (and elsewhere) is functional, but also feels a bit dated. If there's budget (ha!) it might be worth bringing in an illustrator for a couple of days to cleanup the art and establish a clean, consistent style. Otherwise, going with a more iconographic approach might work well.

  • 1.2.1: I love the warning that "Here, we'll take something that's extremely simple and make it more complicated…", since simplicity, alone, is often confused with maintainability—and, of course, in isolation the overhead does initially seem nonsensical.

  • 1.2.2: In Listing 1.3, I appreciate that you use the named attributes, expected and actual; this makes the code self-documenting, and far clearer for people who may not be familiar with the arguments or order of Assert.Equal().

  • 1.4.1: I really appreciate the callout box Dependency Injection or Inversion of Control? as this was something that really confused me when I first encountered the term IoC in this context.

  • 1.4.1: "Always the taxonomist…"—such an appropriate introduction to Martin Fowler!

  • 1.4.4: This section was really useful for bringing the chapter together, and reinforcing the points.

  • Note: Please let me know if any of these suggestions are too nit-picky or unwelcome. Otherwise, I'll continue to provide similar feedback as I read through the second edition—though I imagine I won't have as much from subsequent chapters, since they tend to be a bit more self-contained.

    Jeremy
    Steven van Deursen (32) [Avatar] Offline
    #2
    Hi Jeremy,

    Your feedback is amazing and very welcome. This is a very thorough and well expressed list of comments and suggestions. I've spent the complete morning processing them, and incorporated most of them in the text (it might take some time before those changes are reflected in the MEAP though).

    I didn't feel any of these suggestions where too nit-picky at all. Feel free to provide this kind of feedback to anything else you read.

    Thank you very much for these comments. The second edition became more amazing thanks to you.


    Steven
    Jeremy Caney (28) [Avatar] Offline
    #3
    Steven—I'm so glad to hear that! In that case, I'll certainly keep it coming.
    Francois Genolini (2) [Avatar] Offline
    #4
    Thanks for the MEAP.

    Could you run your source code through static analysis (Microsoft.CodeAnalysis.FxCopAnalyzers NuGet for example)?
    This would allow you to present code that is more aligned with the latest coding guidelines.

    For example Listing 1.1 could see
    <<
    if (writer == null) throw new ArgumentNullException("writer");
    this.writer = writer;
    >>

    replaced with:

    <<
    this.writer = writer ?? throw new ArgumentNullException(nameof(writer));
    >>

    My copy of Visual Studio 2017 suggests this and automatically rewrite my code this way.
    Also could you use a syntax highlighter to present your code?

    Section 1.4.2, p. 31, the listings 1.4 and 1.5 are backwards
    <<
    Listing 1.4 illustrates that we can choose to inject a separate instance into each consumer, whereas listing 1.5 shows that we may alternatively choose to share a single instance across several consumers
    >>
    should be the other way round (1.4 illustrates a shared instance, not 1.5)

    Regards,