This Clean Code Experiment Dedicated to Non-Coders (2)

Posted on December 27th, 2012 by Fernando Zamora

So now it’s been about two weeks since I first wrote part one of this two part series. I only had two comments up to this point and one of them was a reply from me to the first comment. The one comment came from Amy. She pointed out that she liked the shorter cleaner version of the code. However, she wondered if that one could really accomplish everything that the longer nastier version was accomplishing. To point out the short answer: Yes it can and I’m about to explain why and what the benefits are.

The first version attempts to tackle way too many things in the method. Here are some of them (I’m sure that I’ve missed a few).

  • Reads a configuration file
    Reads through the configuration file to find the database connection settings (you know the server, username and password)
  • It then creates a connection object. The connection settings are not sufficient to connect to a back-end database. You must rely on some existing component from either the platform framework or a third party component.
  • Open a connection.
  • Creates a database Adapter
  • Figures out the employee type that we will query for (Hourly or Salaried)
  • Make a query directly against the database with the criteria
  • Read through the data and extract the data into objects formulating a list of Employee type objects
  • many more omitted details

The shorter version hides many of these details and goes directly for only what it needs and nothing more.

Here is a list of problems with the first version (according to coding best practices)

Single Responsibility

Violation of Single Responsibility Principle (a method should do one thing and do it well). SRP is a principle intended for classes not methods. However, if the class should only do one thing, then why should a method within the class be allowed to do more?

Short Methods
Methods should be short(Robert C. Martin). This method is super long because it is doing too much. This makes it harder to read the method for any oncoming programmer (novices and experienced ones). It simply requires an overbearing amount of mental mapping to be able to follow it. I can’t even imagine how much effort it would take to write unit tests to fully test this method. Here is a good discussion (why methods should be short) on the topic from Stack Overflow.

Code Duplication

Violates the rule of avoiding code duplication. Code duplication is where you must write the same code more than once. An example here is the idea of reading the configuration file to create a connection object. That responsibility should be written once and then accessed anywhere else within the code. Supposed that you also have an equipment list instead of an employee list. So you copy and paste the first portion of the code so that you can repeat the connection process. Suppose you have 100 other type of objects that require the same logic to fill from the database. Now (in my Fire Marshall Bill voice) let’s suppose that, for whatever reason, you are now required to get your connection settings from a different file than the config.dat file. That will require you to change 100 plus areas so that yo can properly gather the connection settings. If you’re boss is one that likes a lot of code changes then you win, because you are now required to change 100 plus files. If on the other hand your boss is more mature and likes to minimize risk, you lose. Had you centralized this logic in one component you would need to make a change in one single file. As a matter of fact, in the second example the code would remain exactly as it is now.

Data Abstraction

Violates one of the main tenets of Object Oriented Programming(OOP):abstraction. In other words, it fails to hide implementation details. By doing so, it adds a lot of clutter to the code. This makes it harder to maintain the code because a maintenance programmer will be required to burn a lot of brain power in ensuring that a “code fix” does not have side effects.

Coupling

Violates rule of loose coupling. This code is too tightly coupled to the config.dat file(among other couplings). It is also too tightly coupled to the OracleAdapter. Suppose you are, one day, required to switch to a different database like Microsoft SQL Server. Well now you must change the 100 plus locations where you use this code. Even more importantly, you will not be able to test this code without actually running the application. While it is impossible to unit test 100% of your code, the approach in this example reduces your ability to unit test this code to nearly 0%.

I’m certain there are other violations, but for now I’ll simply stop here. If you wold like to learn more about writing better code, I recommend the following reads:

The concepts here apply to any Object Oriented Language whether it is Java, C++, C# or even VB.net. I can tell you from experience that many programs exist that have been coded as shown in the first example. This type of programming creates problems all the way up the chain; from the lowly programmer all the way up to the organization and finally the customer.

This kind of programming leads to another software engineering concept which is known as technical debt. Think of the programmer that wrote the first example as a credit hungry buyer who rolled out a solution quickly. As the product matures everyone all the way up the chain will feel the pain of debt in the form of slower productivity and the waterbed effect or as others like to call it: the whack-a-mole effect; you hammer one problem and two others pop up as side effects. At some point this debt will have to be paid with interest.

I think there may be a part three of this series to explain how the shorter code can accomplish all the necessary details shown on the first example.

So stay tuned and leave your comments below.

Leave a Reply




Post Comment

Connect With Us

Recent Posts

A Guide for Learning Design Patterns

July 28th 2016 by Fernando Zamora If you’ve been writing code for more than a year, you might have h...

Read More

Using UML to Analyze Legacy Code

June 30th 2016 by Fernando Zamora For every programmer out there creating brand new code and working...

Read More

Python vs. Other Languages

April 29th 2016 by Fernando Zamora For the last two months or so my fellow McLane Advanced Technolog...

Read More

Naming Your Adapter When Implementing the Adapter Pattern

October 19th 2015 by Fernando Zamora At some point in your project you may need to use a third party...

Read More

10 Methods to Help You Debug Legacy Code

September 24th 2015 by Fernando Zamora A large majority of the work we do in our project is to fix r...

Read More