Oren's been creating a lot of discussion on the web recently with his thought provoking and, apparently, controversial blog posts. One of his more recent posts hits on the topic of tools being a weak replacement for a solid understanding of what you're trying to do. I had a bit of a discussion on this with JP when we were at DevTeach and I'm* both of us are firmly on side with Oren. Anyone can teach another programmer a new tool, like nHibernate, but that person won't benefit from the knowledge unless they understand the reason the tool is needed and how it's solving that problem. Although I think knowing the short comings of the tool is important too, I think it falls further down the list.
Today I saw some code that made me cringe. The only reason that I saw the code is because we had a unit test (really it is an integration test masquerading as a unit test) that started failing. I don't understand what the change was that started the failures and that is creating a lot of long term anxiety for me. Once I narrowed the source of the failure down to a small block of code (remember, integration test....so this covers about 10 classes with no mocking) I started to see that people had neglected some of the fundamentals of object oriented programming.
The first thing that I noticed was that we had two classes that were part of a small inheritance chain. They looked something like this.
public class SummaryCustomer
{
private int _customerName;
public int CustomerName
{
get { return _customerName; }
set { _customerName = value; }
}
private Address _mailingAddress;
public Address MailingAddress
{
get { return _mailingAddress; }
set { _mailingAddress = value; }
}
private int _number;
public int Number
{
get { return _number; }
set { _number = value; }
}
}
public class Customer : SummaryCustomer
{
private string _phoneNumber;
public string PhoneNumber
{
get { return _phoneNumber; }
set { _phoneNumber = value; }
}
private Address _mailingAddress;
public Address MailingAddress
{
get { return _mailingAddress; }
set { _mailingAddress = value; }
}
}
The first thing that hit me was that the person naming these functions must not have understood what they were doing. I like to think of inheritance from the "X is a Y" standpoint. In this case it means that the "Customer is a Customer Summary", which to me makes no sense. Regardless, there are other problems here. To me the most egregious is the overriding of the MailingAddress property on the SummaryCustomer class with the exact same implementation in the Customer class. In my IDE (courtesy of ReSharper) I get a warning saying that I should be implement the Customer.MailingAddress instance with the 'new' keyword to override the underlying instance. This is actually what was causing the test to fail.
The code in the test and the code being called from the test looked something like this.
public class SampleTestFixture
{
public void SampleTest()
{
CustomerDal customerDal = new CustomerDal();
Customer customer = customerDal.RetrieveCustomer();
customerDal.DeleteCustomer(customer);
//some assertions
}
}
public class CustomerDal
{
public Customer RetrieveCustomer()
{
//some retrieval code
return new Customer();
}
public void DeleteCustomer(SummaryCustomer customer)
{
//some deleting code
}
}
What happens is that the RetrieveCustomer method returns a Customer typed object. When that object is passed into the DeleteCustomer method as a SummaryCustomer type, things with the MailingAddress method start to go awry. Because of the existence of the same property on the two different types, which also have an inheritance chain, the value in the MailingAddress property immediately after the RetrieveCustomer method call is not available in the DeleteCustomer method.
The easy solution for this was to get rid of the MailingAddress property on the Customer class, but it got me thinking about the things that Oren was getting at in his blog. Developers absolutely need to understand the basics before they start implementing the tools. In this case I consider inheritance to be the tool. It's obvious that the developer (most likely more than one) didn't understand the ramifications of overriding a property, even with ReSharper telling him/her about it.
I see too many devs who are trying to implement things like inheritance without understanding why you use it or how you use it. My experience in the industry is showing that there is a high percentage of developers out there who don't understand the basics of inheritance, polymorphism and encapsulation. Without understanding those three corner stones of OO, you'll only be making them frustrated by showing them things like Dependency Injection, Mocking, or Inversion of Control.
I'm a firm believer in apprenticeship style training. The one-on-one mentoring that plumbers, millwrights and electricians get is a far cry from the learn-on-your-own-through-trial-and-error mentality of our industry. Even in the best case scenarios, junior developers are lucky if they get a few hours a week of mentoring from a competent senior level developer. That kind of timing can't begin to bring a junior developer to the level where they can be introduced to more complicated tools. The exception to this are the developers who invest significant amounts of their own time reading, listening and trying. Then again, this type of person usually rises to the top regardless of industry.
*JP pointed out that I had inadvertantly put him in the tools over fundamentals group by using the term "I'm". Our discussion in Montreal definitely had us both on the fundamentals side of the equation.
posted @ Tuesday, May 29, 2007 7:30 PM