Review of Smells
The concept of ‘Code smells’ was popularized by Kent Beck and Martin Fowler in the book ‘Refactoring: Improving the Design of Existing Code’ (ISBN 978-0201485677). A Code Smell is an indication that something may be wrong in your code. Code Smells are popular because they avoid much of the dogmatic nature of Code Analysis while injecting a sense of humor into what could otherwise be a very dry and adversarial process.
Code smells do not always indicate Code problems. These are not hard and fast rules, more of an indicator that there may be a problem. Smells should be investigated in the early stages before any chance of them getting out of hand.
Using a Code Deodorant
You may be the last one in a team to realize that your code smelled. It is a difficult thing to communicate. It is very delicate matter to have to tell a friend or colleague that their code has a freshness problem. It is also embarrassing to be told that your code leaves a foul stench when it is run through the compiler.
It is far better to apply ‘code deodorant’ to the code yourself before it gets too far. ’Code Deodorant’ is a term I’ll use for the simple steps that we can take to ensure that smells do not crop up in our code unexpectedly. These steps won’t prevent smells from ever forming but they will slow them down so that you can have lead time to take your code back to the showers.
Separation by Interface
Separation by interfaces is a deodorant intended to prevent a broad range of smells that can rise from the way objects interact with each other. The basic process is to separate objects by a well-defined interface and direct all interactions through these interfaces.
Properly structured, these interfaces should add minimal overhead to the implementation. Once implemented, they should simplify ongoing maintenance. If these two goals are not being met, we need to review the best practices listed below.
Interface Best Practices
These interfaces need to have several key characteristics:
- They need to be easily implemented
- They need to use the simplest data types available
- They need to not tie directly to a presentation model
- They need to not tie directly to a specific deployment model
There is also a golden rule when dealing with interfaces. Don’t modify a deployed interface. If new members are needed, derive a new interface and add the new members there.
We want these interfaces to be easily implemented. Implementing them should not add significant implementation overhead to the project. This is more selfish than anything else. If the interface is not easy to implement, it will not likely be implemented and our best designs will be for nothing.
Part of making the interfaces easy to implement requires using the simplest data types available. For example, don’t use a sub class when the base class will work. Also, don’t use a concrete type when an existing interface is available. This widens the net for potential objects to implement our interfaces without having to make substantial changes. It also highlights the prospects for re-usability.
The interfaces can be greatly simplified if you do not tie them to specific UI or deployment models. This also makes it easier to reuse the interfaces in different situations. Looking solely at an Interface, we should not know that it is being used in a Web Application or a WinForm application. We should not be able to tell whether it is a SharePoint application or a DNN application. We should not be able to tell whether the data is stored in SQL Server, Oracle, or web services. If the interface reveals any of these details then the interface is limited to those details.
It is much easier to implement an Interface requiring a string than one requiring a TextBox. It is much easier to implement an Interface requiring a List<BusinessEntities> than one requiring a SQLDataReader.
A Bad Smell: Inappropriate Intimacy
One of the main tenants of object-oriented programming is encapsulation. This means that the implementation details are hidden within the definition of the object. When objects are properly encapsulated, the system as a whole is more resilient to change. When objects violate encapsulation, they smell of inappropriate intimacy, and the system becomes more difficult to change. Problems in one object will more easily propagate to other objects throughout the system and changes in one object may require changes in other objects.
Inappropriate intimacy means that one object knows more about another object than it should. For the sake of your application’s stability, you need to be a prude where your objects are concerned and restrict how they interact.
What Does It Look Like
An easy example of inappropriate intimacy happens with controls in the UI. Whenever a control knows what page it is on, it smells of inappropriate intimacy. If a window knows how any of its controls store or manipulate its data, it smells of inappropriate intimacy.
If the UI knows that the data access layer uses Data Sets, then the UI is too intimate with the data access layer.
If the Business Layer knows that the UI is targeting SharePoint, then the Business Layer is too intimate with the UI.
None of these are bad design decisions. The problem comes when other components start relying on these design decision not changing.
Why Do We Care
Inappropriate intimacy results in applications that are more unstable, are less reusable, and are more likely to have changes in one part of the system impact another part of the system.
When a control knows that it is on a specific page, it cannot easily be moved to another page. If a window uses the knowledge that the Address control stores the city in a text box called txtCity, the window will need to be modified if the control changes how it stores the information.
If the UI knows that the data access layer uses DataSets, then the data access layer cannot change without potentially making changes throughout the UI. We also cannot test the UI or run the UI without a connection to the database to populate the DataSets.
If the Business Layer knows that the UI is targeting SharePoint then the UI cannot be re-targeted to any other platform without potentially needing to rewrite the Business Layer.
In each of these cases, the inappropriate knowledge makes the system brittle. Simple changes create breaking changes. Objects that should be reusable are not because they require that this intimate knowledge not change and continue to be true.
Deodorising the code smell of inappropriate intimacy
Inappropriate intimacy can be solved with appropriate use of Interfaces. As long as two objects interact with each other strictly through published interfaces, there is reduced risk of Inappropriate Intimacy. I say “Appropriate Use of Interfaces” because it is true that the Interfaces can be misused and abused, but for our purposes, we are going to assume that we will follow our best practices in defining our interfaces.
Consider our examples from earlier. It is not OK for a control to know that it is on page MainDataEntry.aspx. If a control must know anything about its containing page, the most it should know is that it implements a given interface. In nearly all cases, it is better for the control to be completely ignorant of the containing page and instead require the containing page to pass data by calling methods or setting properties defined in the interface.
Separation by Interfaces will require the user control to implement a well defined interface in line with our best practices defined earlier.
If the User Control prompts for basic address information, the page should not know what controls are used for the input. The User Control should implement an interface similar to:
|
1 2 3 4 5 6 7 8 9 10 11 12 13 |
public interface IAddressEditor { string AddressLine1 { get; set; } string AddressLine2 {get;set;} string City { get; set; } string State { get; set; } string Zip { get; set; } bool IsPrimary { get; set; } bool IsMailing { get; set; } void DataBind(); void Validate(); } |
As long as the containing page limits the interaction to such an interface, the control is free to change the implementation. The control can decide to use a drop down to prompt for the state, or the control could use a text box. The containing page or any consumer of the interface can remain oblivious to these changes. If the users of the interface make no assumptions as to how this information is stored and used than these two components can be changed without affecting each other. This will improve overall system stability and will allow the user control to be used anywhere that the IAddressEditor is expected. As an added bonus, as long as the containing page stays well behaved, it can use any object implementing the IAddressEditor not just the original control.
Whenever the UI is familiar enough with your data access strategy to know which database platform is storing your data, your objects are too intimate and need to be separated. UI logic or Business logic interacting with a data set or data reader directly makes the logic too intimate with the physical data model. Both of these are disasters waiting to happen. As a general rule, nothing from the System.Data namespaces should ever be exposed beyond the data access layer.
There are an abundance of relational mapping software and strategies available. There is no reason for any application logic to ever be so intimate with the data model that it directly manipulates a dataset.
Separation by Interfaces will require that we define an interface for each entity in our domain model. These interfaces will require a read write property for each field in the underling database object. A simple basic example may include a physical data entity similar to:
For a variety of reasons, such entities will often not conform to our naming conventions. We want to keep the application domain entities loosely coupled to the physical data entities so that we do not have to perpetuate the naming conventions. We also want to make it easier to change the database without having to make substantial changes to our application.
We can keep our domain objects and physical data objects separate through any mapping strategy that we want to use. To meet the requirements or Separation By Interfaces, we will define an interface that our domain objects will implement:
|
1 2 3 4 5 6 7 8 9 10 11 12 13 14 |
public interface IMember { string FirstName { get;set;} string LastName { get;set;} string AddressLine1 { get;set;} string City { get;set;} string State { get;set;} string Zip { get;set;} DateTime DateFirstJoined { get;set;} int MembershipLevel { get;set;} bool BusinessIndicator { get;set;} int ColdCallRotation { get;set;} } |
Our domain object will implement this interface. The implementation of the domain object can handle the mapping from the data object. As long as our code is well behaved and operates exclusively in terms of this interface, then any object implementing this interface can be used without affecting our application logic.
Because there is nothing in the interface to directly tie the domain object back to the original data object, no code that uses the interface needs to be modified should the original data object change. Because there is nothing in the interface to directly tie the domain objet to a given database platform, no code that uses the interface needs to be modified if the database platform change. There is not even anything to limit you to a database at all. Maybe you decide to use web services, or maybe you decide to use XML files Such interfaces give you the ability to structure your application logic in such a way that you don’t have to modify any business logic even for such dramatic changes.
If the only thing that you are expecting to come from the data access layer is objects implementing such simple interfaces, you are not even dependent on the data access layer being in place before you started using its artifacts. Anything that can implement these interfaces will allow you to start developing and testing your business logic.
Conclusion
Despite our best intentions, bad things happen to good code. Even the best code may eventually start to smell over time. There are easy precautions that we can make in our designs to slow these smells down. Such Code Deodorant won’t eliminate all smells, but deodorant will give us more lead time to deal with problems before they get out of hand.
Separation by Interfaces can be a useful deodorant that provides guidance on how to structure our code to limit the impacts of Inappropriate Intimacy and improve the way our objects interact with each other.
Load comments