Before we get directly into Total Cost of Ownership questions, I’d like to give a little background on how I approach this topic. I’m a student of the Theory of Constraints. I’m no expert, but I have a working knowledge of the concepts and how to apply them in Software.
Theory of Constraints (ToC)
The Theory of Constraints is a deep topic. If you’re not familiar with it, I encourage you to read “The Goal,” “The Phoenix Project,” and “The Unicorn Project” as primers. These are all fiction novels that do a fantastic job bringing abstract ideas into concrete reality in a way that’s easy to grasp.
The aspect of the ToC I want to focus on now is the attitude toward inventory and operating expense. When you invest in inventory, you are committing funds that are “frozen” until the end-product is sold. Inventory and Operating Expense detract from the realization of value, i.e., profit. Many managers focus their effort on reducing inventory and operating expense as a way to increase profit. There’s no intrinsic problem with this approach, but it does have some limitations.
First, you need some inventory and some operating expense in order to produce value. This means that the theoretical limit to how much you can reduce inventory and operating expense approaches but can never reach zero. At some point, you will have done all you can.
In ToC, while you are encouraged to reduce inventory an operating expense where it makes sense, this is less important than increasing throughput. If you can produce more quality product faster but incur some minor increase in inventory and operating expense, it’s worth it to do it. ToC’ers are careful to remind you though that local optimizations (e.g, optimizing just one step in the production process) are irrelevant. What matters more is that you can move value through the entire value stream and realize the value as quickly as possible.
ToC in Software
In software engineering, inventory is your backlog. The realization of value is when the software is used. Everything in between is operating expense. The golden metric in software engineering is lead time–the time it takes to deliver a feature from the moment it’s started.
Aside: I have found it helpful to track the delivery time from the moment it's requested (ordered) as well as the time from the moment an engineer starts working on the story. This helps separate engineering bottlenecks from project management bottlenecks.
The activity of software engineering is aimed at delivering value through features. Repairing defects does not add value. They are work that has already been paid for so the repair effort is a net loss to feature delivery. They consume valuable resources (developer time) without adding new value (features).
Three Approaches to Functional Quality
In software delivery, the biggest bottleneck is usually in the testing phase. As it stands, it’s also the phase that most often gets cut. The result is low-quality systems.
In software construction, there are only three approaches to functional quality.
- Production “Testing”. Unfortunately, I’ve worked for some companies that do this. They have no QA and no internal quality gates or metrics. They throw their stuff out there and let the users find the bugs. Even some “Agile” shops do this since it’s easier to teach people how to move post-it notes across whiteboards than it is to teach them how to engineer well.
- Manual Testing. This is much more common. In the worst case, developers write code and pass it through “works on my machine” certification. In the best case companies hire Testers who are integrated with the team. The testers have written test cases that they traverse for each release.
- Automated Testing. This approach is much less common than I would like. In this model, developers write testing programs along with the code they are developing. These tests are run every time changes are committed to check for regressions. The defects slip through, the fixes are captured with additional automated tests so that they don’t recur.
If you are testing in production, you don’t care about quality. Your users will likely care and you are not likely to keep them. Almost everyone understands that this is not an ideal way to proceed. Most people rely on manual testing. Some have some supplemental automated testing. Few have fully reliable automated test suites.
Manual Testing
Many companies rely mostly on manual testing. In a purist’s world, all test cases are executed for every release. Since manual testing–even for small systems–is necessarily time-consuming, most companies do some version of targeted manual testing–targeting the feature that had changes. Of course, defects still slip through, often in the places that weren’t tested because the test cases weren’t considered relevant to the change. What I want to bring your attention to here is not the impact on quality but on lead time.
In this model, when the dev work is done (it’s “dev-complete”), it gets handed off to some QA personnel for manual testing. This person may or may not be on the same team, but it’s irrelevant for our purposes. This person has to get a test environment, setup the software, and march through their manual test cases. This cannot be done in seconds or minutes. In the best case scenario, it takes hours. In reality, it’s usually days. If failures are found the work is sent back to engineering and then process is repeated.
Due to the need to occasionally deploy emergency fixes, there has to be some defined alternative approach to getting changes out that is faster and has less quality gates. Many companies require management and/or compliance approval to use these non-standard processes. Hotfixes themselves have been known to cause outages due to unforeseen consequences of the change that would normally be captured by QA.
Automated Testing, Continuous Integration, and Continuous Deployment
In contrast to the manual testing approach, automated testing facilitates rapid deployment. The majority of use-cases are covered by test programs that run on every change. The goal is to define the testing pipeline in such a way that passing it is a good enough indicator of quality that the release should not be held up.
In this model, branches are short-lived and made ready to release as quickly as possible. The test cases are executed by a machine which takes orders of magnitude less time than a human being. Once the changes have passed the automated quality gates, they are immediately deployed to production, realizing the value for the business.
When done well, this process takes minutes. Even with human approval requirements, I’ve had lead times of less than an hour to get changes released to production.
The capabilities that these processes enable are enormous. Lead times go way down which means higher feature throughput for our engineering teams. We are able to respond to production events more quickly which increases agility not only for our engineering teams but also for our businesses. We have fewer defects which means even more time to dedicate to features.
DevOps
Many engineers think of DevOps as automating deployments. That’s certainly part of it, but not all. DevOps is about integrating your ops and dev teams along the vertical slices. Software construction should be heavily influenced by operational concerns. If the software is not running, then we are not realizing value from it. Again, the ToC mindset is helpful here.
Software construction should include proper attention to logging, telemetry, architecture, security, resiliency, and tracing. Automating the deployments allows for quickly fine-tuning these concerns based on the team’s experience running the service in production.
Deployment automation is a good first step and helps with feature-delivery lead times right away. Let’s think about some other common sources of production service failure:
- Running out of disk space.
- Passwords changed.
- Network difficulties.
- Overloaded CPU.
- Memory overload.
- Etc…
A good DevOps/SRE solution would monitor for these (and other) situations and alert engineers before they take down the service. In the worst case, they would contain detailed information about the problem and what to do to address it. This reduces downtime for the service and allows you to restore service faster in the case of an outage. From a ToC perspective, both outcomes increase the time you are realizing value from the software.
So Why Are Modern Engineering Practices Still Relatively Rare in our Industry?
I’ve been trying to answer this question for 17 years. I think I finally have a handle on it.
Remember that it’s common to attempt to increase profit by reducing operating expense. Automated testing and deployment requires a fair amount of expertise and a not-small amount of time to setup and do well. They are not often regarded as “features” even though the capability of rapid, confident change certainly is. These efforts begin as a significant increase in operating expense, especially if it’s being introduced into a brown-field project for the first time.
Aside: It can be hard to convince managers that we should spend time cleaning up technical debt. It's harder to convince them later that failing to clean up technical debt is the reason it takes so long to change the text in an email template. Managers want the ability to change software quickly, but they don't always understand the technical requirements to do that. Treating lead time like a first-class feature and treating defects as demerits to productivity can help create a common language between stakeholders about where it's important to spend engineering time. If you can measure lead time, you can show your team getting more responsive to requests and delivering faster.
The cost of getting started with modern engineering practices is even bigger than it first appears. It is not possible to build fast, reliable automated tests without learning a range of new software engineering principles, patterns, and practices. These include but are not limited to Test Driven Development, Continuous Integration, Continuous Deployment, Design Patterns, Architectural Patterns, Observability patterns, etc.. Many software engineers and managers alike balk at this challenge, not seeing what lies on the other side. Most engineers will slow down when learning how to practice these things well since the patterns are unfamiliar and the tendency toward old habits is strong. Many will declare automated testing a waste of time since it doesn’t work well with what they’ve always done. The idea that they may have to change the way they develop is alien to them and not seriously considered. The promise is increased productivity, but the initial reality is the opposite– a near work-stoppage. This is true unless you are working with engineers who’ve already climbed these learning curves.
Engineers will describe it as “this takes too long.” Managers will be frustrated by the delays to their features. In business terms, this is seen as increased operating expense and lower throughput–the opposite of what we want. We are inclined as an industry to abandon the effort. We feel justified in doing so based on the initial evidence.
This is a mistake.
All of these costs are mitigated enormously if these efforts are done at the beginning of the project. Very often companies will create mountains of technical debt in the name of “moving fast.” These companies will pay an enormous cost when it’s time to harden their software engineering and delivery chops. The irony is that the point of modern engineering practices is to facilitate going fast, so this argument should be viewed skeptically. There are cases when this tradeoff is warranted, to be sure, but it is my opinion that this is less often than is commonly believed.
Getting Through the Learning Dip
We must remember that we’re not as unique as we think we are. Learning new ways of operating is hard. We can look to the experience of other enterprises to remind us why we’re doing this. It’s clear from the data that companies that embrace modern engineering practices dramatically reduce their lead times and the total cost of ownership of their software assets. If we want to compete with them we must be willing to climb this initial learning curve.
The frustration and anxiety we feel when we take on these challenges is so normal it has a name: “The Learning Dip.” We must recognize that this is where we are and keep going! It’s important not to abandon the effort. For those who like to be “data driven,” tracking lead time will be helpful. For project managers, treating defects as a negative to productivity will also help drive the right attention to quality. Again, time spent fixing bugs is time not spent building out new features. Defects as a percentage of your backlog is something you can measure and show to indicate progress to your stakeholders.
I once managed a team that did one release every 5-6 weeks. After investing heavily in this learning, we were able to release three times in one week. It was a big moment for us and represented enormous progress, but the goal was to be able to release on-demand. We celebrated, but we were not satisfied. The overall health of our service began climbing rapidly according to metrics chosen by our business stakeholders. More than one of the engineers told me later that “I will never go back to working any other way.” They haven’t.
As engineers, even the most experienced people must be willing to adopt a learner’s stance (or “growth mindset”). We must change our design habits to enable automated testing and delivery. We must learn to care about the operational experience of our software and about getting our features into production as fast as possible without defects. Any regular friction we encounter during the testing and deployment process should be met with aggressive action to fix and/or automate away the pain.
As managers, we must set the expectation that our engineers will learn and practice all of the modern software engineering techniques. This includes TDD, CI, CD, DevOps, and SRE concepts. We must make time for them to do so and protect that time.
If we are concerned about the initial impact to our timelines, we can hire engineers who already have this expertise to help guide the effort. It is not necessary that every engineer has the expertise already, but it is necessary that those who have it can teach it to the others and those who don’t are actively engaged in learning. This will dramatically reduce time spent in The Learning Dip in the early stages of rewiring how our teams think about their solutions. If we can’t afford to hire FTE’s for this role, perhaps we can find budget to hire experienced consultants to work with us and get us through the slump.
Conclusion
Modern Engineering Practices do represent a significant initial expense for teams just learning how to employ them. However, this initial expense enables a force multiplicative effect on feature delivery. In other words, it’s true that these techniques cost more–at least initially. It’s also true that they reduce the TCO of your software assets over the long-term. They speed up your engineering teams’ and business’ ability to react to the marketplace. A little more expense up front will save you a lot more down the road. As Uncle Bob says, “the only way to go fast is to go well.”
Go well and be awesome.
Testing software is critically important to ensuring quality. Automated tests provide a lower Mean Time to Feedback (MTTF) for errors as well as enable developer’s to make changes without fear of breaking things. The earlier in the SDLC that errors can be detected and corrected, the better. (See the Test Pyramid). As engineers on the platform we should practice TDD in order to generate a thorough bed of unit tests. Unit tests alone do not ensure that everything works as expected so we will need gradually more sophisticated forms of testing.
There are different approaches to testing software. This document chooses to articulate types of automated testing by the point in the SDLC at which it is executed and by what it covers. There may be different strategies for testing at each of these lifecycle points (e.g., deterministic, fuzz, property-based, load, perf, etc..)
SDLC Stage | Type | Target | Actors | Description |
---|---|---|---|---|
Design / Build Time | Unit | Component | Engineer, CI | In process, no external resources. Mock at the Architectural boundaries but otherwise avoid mocks where possible. |
Integration | Component | Engineer, CI | These tests will mostly target the adapters for external systems (e.g., file io, databases, 3rd party API’s, 1st party API’s that are not the component under test.) Integration tests are not written against real instances of external systems beyond the control of the component in question. | |
Post-Deployment to Test Environment | Acceptance | Platform | CD | Largely black box, end-to-end testing. Acceptance tests will run against a live running instance of the entire system. |
Operational Tests | Platform | CD |
|
|
Manual UX Testing | Platform | Engineering, UX, QA, etc. | This testing is qualitative and pertains to the “feel” of the platform with respect to the user experience. | |
Post-Production Release | Smoke | Platform | Engineer | A small suite of manual tests to validate production configuration. |
Synthetic Transactcions | Platform | Automated | Black box, end-to-end use-case testing, automated, safe for production. These tests are less about correctness and more about proving the service is running. | |
This list is not exhaustive, but it does represent the more common cases we will encounter. |
Testing Pyramid
In general, our heaviest investment in testing should be done at the time the code is written. This means that unit tests should far outweigh other testing efforts. Why?
Unit tests are very low-cost to write and have very low Mean Time to Feedback (MTTF). This means they have the greatest ROI of any other kind of test.
This emphasis on unit testing is often represented as a pyramid
TDD
TDD is the strongly preferred manner of writing unit tests as it ensures that all code written is necessary (required by a test) and correct. Engineers who are not used to writing code in a TDD style often struggle with the practice in the early stages. If this describes your experience, be satisfied with writing tests for the code you’ve written in the same commit.
The activity of TDD consists of three steps:
- (RED) Write a failing unit test.
- (GREEN) Write enough production code to make it pass.
- (REFACTOR) Now make the code pretty.
The unit tests you write should strive to obey the three laws of TDD:
- Don’t write any production code unless it is to make a failing unit test pass.
- Don’t write any more of a unit test than is sufficient to fail; and compilation failures are failures.
- Don’t write any more production code than is sufficient to pass the one failing unit test.
Good unit tests have the following attributes:
- The test must fail reliably for the reason intended.
- The test must never fail for any other reason.
- There must be no other test that fails for this reason.
These are ideals and practicing TDD this way is often difficult for newcomers to the practice. If this describes you then try scaling back to submitting the unit tests in the same commit as your production code. Don’t forget to commit early and often!
Further Reading
It’s impossible to fully convey the scope of what you should know about test automation in this document. Below are some resources you may be interested in as you move through your career.
- Test Driven Development: By Example by Kent Beck
- The Art of Unit Testing: 2nd Edition by Roy Osherove
- Working Effectively With Legacy Code by Michael Feathers
- Refactoring: Improving the Design of Existing Code (2nd Edition) by Martin Fowler
- Performance vs. Load vs. Stress Testing
Testing software is critically important to ensuring quality. Automated tests provide a lower Mean Time to Feedback (MTTF) for errors as well as enable developer’s to make changes without fear of breaking things. The earlier in the SDLC that errors can be detected and corrected, the better. (See the Test Pyramid). As engineers on the platform we should practice TDD in order to generate a thorough bed of unit tests. Unit tests alone do not ensure that everything works as expected so we will need gradually more sophisticated forms of testing.
There are different approaches to testing software. This document chooses to articulate types of automated testing by the point in the SDLC at which it is executed and by what it covers. There may be different strategies for testing at each of these lifecycle points (e.g., deterministic, fuzz, property-based, load, perf, etc..)
SDLC Stage | Type | Target | Who Runs Them? | Description |
Design / Build Time | Unit | Single Application | Engineer, CI | In process, no external resources. Mock at the Architectural boundaries but otherwise avoid mocks where possible. |
Integration | Single Application | Engineer, CI | These tests will mostly target the adapters for external systems (e.g., file io, databases, 3rd party API’s, 1st party API’s that are not the component under test.) Integration tests differ from acceptance tests in that they should never fail to an issue with an external service. | |
Post Deployment to Test Environment | Acceptance | Entire System or Platform | CI, CD | Largely black box, end-to-end testing. For bonus points, tie failures into telemetry to see if your monitors are alerting you. |
Manual UX Testing | Entire System or Platform | Engineer, QA, Users | This testing is qualitative and pertains to the “feel” of the platform with respect to the user experience. | |
Post Production Release | Smoke | Entire System or Platform | Engineer, CD | A small suite of manual tests to validate production configuration. |
Synthetic Transactions | Entire System or Platform | System | Black box, end-to-end use-case testing, automated, safe for production. These tests are less about correctness and more about proving the service is running. | |
Other? | This is not an exhaustive list. |
Emphasize Unit Tests
In general, our heaviest investment in testing should be done at the time the code is written. This means that unit tests should far outweigh other testing efforts. Why?
Unit tests are very low-cost to write and have very low Mean Time to Feedback (MTTF). This means they have the greatest ROI of any other kind of test.
The other kinds of testing are important but they get more complex as you move through the SDLC. This makes covering finicky edge-cases challenging from both an implementation and maintenance perspective. Unit Tests don’t have these drawbacks provided you follow good TDD guidance.
TDD
TDD is the strongly preferred manner of writing unit tests as it ensures that all code written is necessary (required by a test) and correct. Engineers who are not used to writing code in a TDD style often struggle with the practice in the early stages. If this describes your experience, be satisfied with writing tests for the code you’ve written in the same commit until it starts to feel natural.
The activity of TDD consists of three steps:
- (RED) Write a failng unit test.
- (GREEN) Write enough productino code to make it pass.
- (REFACTOR) Now make the code pretty.
The unit tests you write should strive to obey the three laws of TDD:
- Don’t write any production code unless it is to make a failing unit test pass.
- Don’t write any more of a unit test than is sufficient to fail; and compilation failures are failures.
- Don’t write any more production code than is sufficient to pass the one failing unit test.
Good unit tests have the following attributes:
- The test must fail reliably for the reason intended.
- The test must never fail for any other reason.
- There must be no other test that fails for this reason.
Further Reading
It’s impossible to fully convey the scope of what you should know about test automation in this document. Below are some resources you may be interested in as you move through your career.
- Test Driven Development: By Example by Kent Beck
- The Art of Unit Testing: 2nd Edition by Roy Osherove
- Working Effectively With Legacy Code by Michael Feathers
- Refactoring: Improving the Design of Existing Code (2nd Edition) by Martin Fowler
I was reading this stack overflow question: How can I solve this: Nhibernate Querying in an n-tier architecture?
The author is trying to abstract away NHibernate and is being counseled rather heavily not to do so. In the comments there are a couple of blog entries by Ayende on this topic:
The false myth of encapsulating data access in the DAL
Architecting in the pit of doom the evils of the repository abstraction layer
Ayende is pretty down on abstracting away NHIbernate. The answers on StackOverflow push the questioner toward just standing up an in-memory Sqlite instance and executing the tests against that.
The Sqlite solution is pretty painful with complex databases. It requires that you set up an enormous amount of data that isn’t really germane to your test in order to satisfy FK and other constraints. The ceremony of creating this extra data clutters the test and obscures the intent. To test a query for employees who are managers, I’d have to create Departments and Job Titles and Salary Types etc., etc., etc.. Dis-like.
What problem am I trying to solve?
In the .NET space developers tend to want to use LINQ to access, filter, and project data. NHibernate (partially) supports LINQ via an extension method off of ISession. Because ISession.Query<T> is an extension method, it is not stubbable with free mocking tools such as RhinoMocks, Moq, or my favorite: NSubstitute. This is why people push you to use the Sqlite solution—because the piece of the underlying interface that you want to use most of the time is not built for stubbing.
I think that a fundamental problem with NHibernate is that it is trying to serve 2 masters. On the one hand it wants to be a faithful port of Hibernate. On the other, it wants to be a good citizen for .NET. Since .NET has LINQ and Java doesn’t, the support for LINQ is shoddy and doesn’t really fit in well the rest of the API design. LINQ support is an “add-on” to the Java api, and not a first-class citizen. I think this is why it was implemented as an extension method instead of as part of the ISession interface.
I firmly disagree with Ayende on Generic Repository. However, I do agree with some of the criticisms he offers against specific implementations. I think his arguments are a little bit of straw man, however. It is possible to do Generic Repository well.
I prefer to keep my IRepository interface simple:
public interface IRepository : IDisposable { IQueryable<T> Find<T>() where T: class; T Get<T>(object key) where T : class; void Save<T>(T value) where T: class; void Delete<T>(T value) where T: class; ITransaction BeginTransaction(); IDbConnection GetUnderlyingConnection(); }
Here are some of my guidelines when using a Generic Repository abstraction:
- My purpose in using Generic Repository is not to “hide” the ORM, but
- to ease testability.
- to provide a common interface for accessing multiple types of databases (e.g., I have implemented IRepository against relational and non-relational databases) Most of my storage operations follow the Retrieve-Modify-Persist pattern, so Find<T>, Get<T>, and Save<T> support almost everything I need.
- I don’t expose my data models outside of self-contained operations, so Attach/Detach are not useful to me.
- If I need any of the other advanced ORM features, I’ll use the ORM directly and write an appropriate integration test for that functionality.
- I don’t use Attach/Detach, bulk operations, Flush, Futures, or any other advanced features of the ORM in my IRepository interface. I prefer an interface that is clean, simple, and useful in 95% of my scenarios.
- I implemented Find<T> as an IQueryable<T>. This makes it easy to use the Specification pattern to perform arbitrary queries. I wrote a specification package that targets LINQ for this purpose.
- In production code it is usually easy enough to append where-clauses to the exposed IQueryable<T>
- For dynamic user-driven queries I will write a class that will convert a flat request contract into the where-clause needed by the operation.
- I expose the underlying connection so that if someone needs to execute a sproc or raw sql there is a convenient way of doing that.
I recently had a subtle production bug introduced by creating more than one Ninject binding for a given interface to the same instance.
I wanted to be able to see what bindings existed for a given interface, but Ninject does not provide an easy way to do that.
This gist contains an extension method I wrote (with the help of a StackOverflow article) to acquire this information.
As this code relies on using reflection to get a private member variable, this code is brittle in the face of a change in the implementation of KernelBase. In the meantime, it works on my machine.
I just learned that you could do this:
Update: 2013-01-25
Note that successive chained mocking calls to RhinoMocks fail. I now have a reason to prefer NSubstitute other than it’s beautifully simple API.
One of the points I tried to make in my talk about TDD yesterday is that TDD is more focused on the clarity and expressiveness of your code than on its actual implementation. I wanted to take a little time and expand on what I meant.
I used a Shopping Cart as an TDD sample. In the sample, the requirement is that as products are added to the shopping cart, the cart should contain a list or OrderDetails that are distinct by product sku. Here is the test I wrote for this case (this is commit #8 if you want to follow along):
[Test] public void Details_AfterAddingSameProductTwice_ShouldDefragDetails() { // Arrange: Declare any variables or set up any conditions // required by your test. var cart = new Lib.ShoppingCart(); var product = new Product() { Sku = "ABC", Description = "Test", Price = 1.99 }; const int firstQuantity = 5; const int secondQuantity = 3; // Act: Perform the activity under test. cart.AddToCart(product, firstQuantity); cart.AddToCart(product, secondQuantity); // Assert: Verify that the activity under test had the // expected results Assert.That(cart.Details.Count, Is.EqualTo(1)); var detail = cart.Details.Single(); var expectedQuantity = firstQuantity + secondQuantity; Assert.That(detail.Quantity, Is.EqualTo(expectedQuantity)); Assert.That(detail.Product, Is.SameAs(product)); }
The naive implementation of AddToCart is currently as follows:
public void AddToCart(Product product, int quantity) { this._details.Add(new OrderDetail() { Product = product, Quantity = quantity }); }
This implementation of AddToCart fails the test case since it does not account for adding the same product sku twice. In order to get to the “Green” step, I made these changes:
public void AddToCart(Product product, int quantity) { if (this.Details.Any(detail => detail.Product.Sku == product.Sku)) { this.Details.First(detail => detail.Product.Sku == product.Sku).Quantity += quantity; } else { this._details.Add(new OrderDetail() { Product = product, Quantity = quantity }); } }
At this point, the test passes, but I think the above implementation is kind of ugly. Having the code in this kind of ugly state is still a value though because now I know I have solved the problem correctly. Let’s start by using Extract Condition on the conditional expression.
public void AddToCart(Product product, int quantity) { var detail = this.Details.SingleOrDefault(d => d.Product.Sku == product.Sku); if (detail != null) { detail.Quantity += quantity; } else { this._details.Add(new OrderDetail() { Product = product, Quantity = quantity }); } }
The algorithm being used is becoming clearer.
- Determine if I have an OrderDetail matching the Product Sku.
- If I do, increment the quantity.
- If I do not, create a new OrderDetail matching the product sku and set it’s quantity.
It’s a pretty simple algorithm. Let’s do a little more refactoring. Let’s apply Extract Method to the lambda expression.
public void AddToCart(Product product, int quantity) { var detail = GetProductDetail(product); if (detail != null) { detail.Quantity += quantity; } else { this._details.Add(new OrderDetail() { Product = product, Quantity = quantity }); } } private OrderDetail GetProductDetail(Product product) { return this.Details.SingleOrDefault(d => d.Product.Sku == product.Sku); }
This reads still more clearly. This is also where I stopped in my talk. Note that it has not been necessary to make changes to the my test case because the changes I have made go to the private implementation of the class. I’d like to go a little further now and say that if I change the algorithm I can actually make this code even clearer. What if the algorithm was changed to:
- Find or Create an OrderDetail matching the product sku.
- Update the quantity.
In the first algorithm, I am taking different action with the quantity depending on whether or not the detail exists. In the new algorithm, I’m demoting the importance of whether the order detail already exists so that I can always take the same action with respect to the quantity. Here’s the naive implementation:
public void AddToCart(Product product, int quantity) { OrderDetail detail; if (this.Details.Any(d => d.Product.Sku == product.Sku)) { detail = this.Details.Single(d => d.Product.Sku == product.Sku); } else { detail = new OrderDetail() { Product = product }; this._details.Add(detail); } detail.Quantity += quantity; }
The naive implementation is a little clearer. Let’s apply some refactoring effort and see what happens.. Let’s apply Extract Method to the entire process of getting the order detail.
public void AddToCart(Product product, int quantity) { var detail = GetDetail(product); detail.Quantity += quantity; } private OrderDetail GetDetail(Product product) { OrderDetail detail; if (this.Details.Any(d => d.Product.Sku == product.Sku)) { detail = this.Details.Single(d => d.Product.Sku == product.Sku); } else { detail = new OrderDetail() { Product = product }; this._details.Add(detail); } return detail; }
This is starting to take shape. However, “GetDetail” does not really communicate that we may be creating a new detail instead of just returning an existing one. If we rename it to FindOrCreateOrderDetailForProduct, we may get that clarity.
public void AddToCart(Product product, int quantity) { var detail = FindOrCreateDetailForProduct(product); detail.Quantity += quantity; } private OrderDetail FindOrCreateDetailForProduct(Product product) { OrderDetail detail; if (this.Details.Any(d => d.Product.Sku == product.Sku)) { detail = this.Details.Single(d => d.Product.Sku == product.Sku); } else { detail = new OrderDetail() { Product = product }; this._details.Add(detail); } return detail; }
AddToCart() looks pretty good now. It’s easy to read, and each line communicates the intent of our code clearly. FindOrCreateDetailForProduct() on the other hand is less easy to read. I’m going to apply Extract Conditional to the if statement, and Extract Method to each side of the expression. Here is the result:
private OrderDetail FindOrCreateDetailForProduct(Product product) { var detail = HasProductDetail(product) ? FindDetailForProduct(product) : CreateDetailForProduct(product); return detail; } private OrderDetail CreateDetailForProduct(Product product) { var detail = new OrderDetail() { Product = product }; this._details.Add(detail); return detail; } private OrderDetail FindDetailForProduct(Product product) { var detail = this.Details.Single(d => d.Product.Sku == product.Sku); return detail; } private bool HasProductDetail(Product product) { return this.Details.Any(d => d.Product.Sku == product.Sku); }
Now I’ve noticed that HasProductDetail and FindDetailForProduct are only using the product sku. I’m going to change the signature of these methods to accept only the sku, and I’ll change the method names accordingly.
public void AddToCart(Product product, int quantity) { var detail = FindOrCreateDetailForProduct(product); detail.Quantity += quantity; } private OrderDetail FindOrCreateDetailForProduct(Product product) { var detail = HasDetailForProductSku(product.Sku) ? FindDetailByProductSku(product.Sku) : CreateDetailForProduct(product); return detail; } private OrderDetail CreateDetailForProduct(Product product) { var detail = new OrderDetail() { Product = product }; this._details.Add(detail); return detail; } private OrderDetail FindDetailByProductSku(string productSku) { var detail = this.Details.Single(d => d.Product.Sku == productSku); return detail; } private bool HasDetailForProductSku(string productSku) { return this.Details.Any(d => d.Product.Sku == productSku); }
At this point, the AddToCart() method has gone through some pretty extensive refactoring. The basic algorithm has been changed, and the implementation of the new algorithm has been changed a lot. Now let me point something out: At no time during any of these changes did our test fail, and at no time during these changes did our test fail to express the intended behavior of the class. We made changes to every aspect of the implementation: We changed the order of the steps in the algorithm. We constantly added and renamed methods until we had very discrete well-named functions that stated explicitly what the code is doing. The unit test remained a valid expression of intended behavior despite all of these changes. This is what it means to say that a test is more about API than implementation. The unit-test should not depend on the implementation, nor does it necessarily imply a particular implementation.
Happy Coding!
Tomorrow I will be giving a talk on TDD at CMAP. The demo code and outline I will be using can be found on bitbucket here.
Here is the outline for the talk:
- I. Tools
- A. Framework
- B. Test Runner
- C. Brains
- II. Test Architecture
- A. Test Fixture
- B. Setup
- C. Test Method
- D. TearDown
- III. Process
- A. Red
- B. Green.
- C. Refactor.
- D. Rinse and Repeat.
- IV. Conventions
- A. At least one testfixture per class.
- B. At least one test method per public method.
- C. Test Method naming conventions
- i. MethodUnderTest_ConditionUnderTest_ExpectedResult
- D. Test Method section conventions
- i. Arrange
- ii. Act
- iii. Assert
- V. Other Issues
- A. Productivity Study
- B. Testing the UI
- i. Not technically possible without more tooling/infrastructure
- ii. MVC patterns increate unit-test coverage.
- iii. Legacy code.
- a. Presents special problems.
- b. Touching untested legacy code is dangerous.
- c. Boy-Scout rule.
- d. Use your own judgment
- C. Pros and Cons
- i. Pros
- a. Quality.
- b. Encourage a more loosely-coupled design.
- c. Document the work that is done.
- d. Regression testing.
- e. Increased confidence in working code means changes are easier to make.
- f. Encourages devs to think about code in terms of API instead of implementation.
- 1. Makes code more readable.
- 2. Readable code communicates intent more clearly.
- 3. Readable code reduces the need for additional non-code documentation.
- ii. Cons
- a. Takes longer to develop.
- b. Test code must be maintained as well.
- c. Requires that devs adapt to new ways of thinking about code.
- D. Notes
- i. You’re already doing it.
- ii. "The Art of Unit Testing" by Roy Osherove
- iii. "Clean Code" by Robert C. Martin
- iv. "Head First Design Patterns” by Elizabeth and Eric Freeman, Bert Bates, and Kathy Sierra