KUP Assessments – Implementation Diary -TDD & Refactoring

The next few posts are going to be more like diary entries than posts on specific topics. They will have show my train of thought and therefore will be full of assumptions.

Last year I did a course of TDD with Jpassion.com. It was a useful course and allowed me to also refine and expand on many of my projects. However it did not cover when starting projects you start creating tests. My first instinct was to start straight away however it meant an a lot of mocking and from my searches of the net, there are mixed feelings regarding mocking.

So I have settled on a actual sql database for building the account database as I can also be assured that the SQL statements work. I have had a quandary of refactoring. Consider the following statements:

public Boolean createAccount(String username, String email) throws StoringAccountException, AccountAlreadyExistsException{
connect();
if (checkAccountAlreadyExists(username)) {
close();
throw new AccountAlreadyExistsException();
}
try {
conn.prepareStatement(generateNewAccountSQLStatement(username, email)).execute();
close();
return true;
} catch (SQLException e) {
e.printStackTrace();
close();
throw new StoringAccountException();
}
}


public boolean updateAccountEmail(String username, String updatedEmail) {
 connect();
     if(!checkAccountAlreadyExists(username)) {
      close();
      return false;
     } 
     try {
 conn.prepareStatement(generateUpdateAccountStatement(username, updatedEmail)).execute();
 } catch (SQLException e) {
 // TODO Auto-generated catch block
 e.printStackTrace();
 close();
 return false;
 }
     close();
     return true;
 }
public boolean deleteAccount(String username) {
 connect();
      if(!checkAccountAlreadyExists(username)) {
       close();
 return false;
 }
      try {
   conn.prepareStatement(generateDeleteAccountStatement(username)).execute();
   return true;
   } catch (SQLException e) {
   // TODO Auto-generated catch block
   e.printStackTrace();
   close();
   return false;
   }
 
      
 }

Now I could refactor these as there is a some repetition here. However, some of the methods throw specific exceptions. How do I refactor and keep the exceptions? Or is the issue that some of methods throw exceptions and some don’t. A lack consistency in the interface. Should all the methods throw an exception which may make it easier to refactor? These crud methods however may change further and refactoring could complicate adding the functionality. I will have to return to this issue later and see if I can refactor once other functionality e.g SQL injection prevention is included. I can of course use a finally clause to tidy up the code.

KUP Assessments – An ‘Assessment Creator’ Creates an Account

This requirement relates to the following classes with associated test cases:

  • KupGatehouse
  • Authentication Manager
  • KupFacade
  • KupAssessments
  • AccountManager
  • AccountDatabase
  • HTTP_HeaderChecker

Additionally there is a bash script to test from the frontend of a working system.

Changing Names to Suit Interfaces.

A number of interfaces have altered their names, slightly to reflect that they are interfaces. e.g AccountDatabase has become IaccountDatabase.

Database choice – MySQL

The first implementation of the IaccountsDatabase will be an MySQL database. this is because of the support that is available for MySQL and for not for speed or ideological reasons. I just want it to work.

Database setup

The initial database, which is not embedded so there is flexibility in where the database is located, is set up using a sql scipt. The login details for the AccoundDatabaseSQL class uses to access the database is kept in a properties file on the server. I do worry over the security issues for this but for lack of finding a better solution and the fact that if someone able to access the file, there will be greater problems that just the accounts database being compromised. This will require revisiting later.

IAccount Class creation

My first thoughts are to use a factory pattern for the creation and submission of accounts. The problem is I would be restricted to what the data structure of the concrete account classes would be. Behavior of course would be malleable, but what if there was a method of a class reveal structure a run time. e.g revealDataStructure() it would be up to the IaccountDatabase to store this in its database. The database would also need to be able to handle the structure. In the case of MySQL database this is completely reasonable. so how do I get the factory to insert the data structure. Well I could instead use a method for Iaccount such as loadDataStructure(Map aMap) the class would then wire up its data structure itself. It could also have getDataStructure() which would return a map which it’s keys and values (Strings) could be iterated through to store its data structure. Or should I be creating a IAccount class at all. It is only going to be flattened into a xml or json structure anyway? No. For simplicity of the interface between components, the Iaccout being passed around would be better.

Refactoring Desktop Snowfall

Using the refactoring checklist I will attempt to improve the design of  my Desktop Snowfall project

  • Can the code be Tested?  – The application is GUI based application with not a huge amount going on underneath.  Also some of this is subjective ie is the snow realistic? So it is impossible to test for. The current class diagram looks likes this:

  • Do you need a seam?
    • extract methods using protected not private that use the seam.
  • Create Tests

To create the tests I will need to extract the GUI from the other logic e.g generation of the snow. I started by creating an Environment package and an Environment interface that can store the environmental variables, screen size etc. This is then injected into the snowfall class. Additionally a factory pattern could be used for creating the items in the weather. Hence as weather factory interface could created and injected into the snowfall class. For future flexability objects that fall are defined by a Map held by the environment. Keys in the map are the class names of the objects and the int is the percentage occurrence in the environment.  Therefore the amount of objects created is still controlled by the user through the variable renamed from amountOfAnow to theAmountOfWeatherObjects.

I then created implementations and tests for them and I then integrated them into the design.

  • Start with the deepest branch to shortest branch – Refactored
  • Keep changes small and consistently retest. – Refactored
  • Remove complexity – Refactored
  • Methods should not have more than 10 lines of code – Refactored with a couple of instances where a method is 12 lines of code.
  • Replace conditionals with strategy pattern. – Refactored – achieved with introduction of weatherObjects, WeatherFactory and Enviroment interfaces.
  • Remove local variables – Refactored
  • Remove comments – use descriptive method names. – Refactored
  • Remove cryptic or clever code, – Refactored
  • Remove abbreviated code – Refactored
  • Refactor with dependency injection – Refactored used with implementing strategy pattern.
  • Refactor using SOLID
    • Single Responsibility Principle (SRP)
      • All classes now have singular responsibilities
    • Open Closed Design Principle
      • Up to this point I have amended, restructured the program to fit in with design principles. One I have completed this list. I will hold to this more strictly. Which begs the question. Should it have come first or last.
    • Liskov Substitution Principle (LSP)
      • implemented with the WeatherFactory, Environment, WeatherObject and Weather classes.
    • Interface Segregation Principle
      • At present the interfaces are basic and largly singular in purpose. e.g Environment interface only for retreiving details about the weather.
    • Dependency Inversion Principle
      • Achieved by making Environment, Weather and WeatherFactory dependent on abstractions.
  • Refactor using other design principles
    • Encapsulate what changes.
      • Reviewed – no further changes needed.
    • Don’t Repeat Yourself (DRY)
      • Reviewed. No further changes needed
    • Don’t look for things. Ask for things. (Dependency Injection)
      • Reviewed. No further changes needed.
    • Favor Composition over Inheritance
      • Reviewed. No further changes needed
    • Program to the Interface not to the Implementation
      • Reviewed. No further changes needed
    • Delegation Principles
      • Reviewed. No further changes needed

Now the structure of the program now looks like this:

I will update the source forge and the projects page in the coming days.

Refactoring – Liskov Substitution Principle (LSP) Applied to JobMan

In the previous article I introduced 2 base classes Job and SpawnableJob. Their purpose was to forfill the closed open principle. However, as a side effect when all jobs are returned from the database spawnablejobs was also returned. This breaks the LSP as it is not intended that SpawnableJobs are substutable for Jobs and to do so would violate the single responsibility principle.

So to fix this. Spawnable will change so it has a filed of jobToBeReplicated  which will hold an instance of Job and will no longer inherit from Job.

You Make Me Feel Like Testing, Gonna Test The Night Away

I have reached the stage where I no longer wish to mess with the structure of the WhitestarMediaLibrary. In art, you have to decide when to stop and move on. In this case I may wish to alter the structure if, in the future a fundamental fault is identified. Now I feel like I need to brush up on my testing skills.

However before I do this, below is a complete class diagram of the WhitestarsMediaLibrary.

 

I have also created a git repository on GitLab which I will make public once I have completed adding tests and documentation.

 

Let’s get on with testing.

As I am pretty keen on all things GTT (Getting Things Done),  I thought a checklist would be appropriate when going through each class. I imagine this list will grow and change as I complete creating the tests on each class. As of today the checklist is:

  • No test for those to simple to fail (e.g setters and getters)
  • Test positive and negative results
  • Test Exceptions
  • Suitalble for Test Parameterisation
  • Test Documentation

WhitestarMediaLibrary – database management, Oh CRUD.

As part of implementing the WhitestarMediaLibary project once it had been reviewed using SOLID principles, In this post I will cover adding a database interface and implementation.

The first thing that I realised when I came to implement the database interface is I had not considered the update and delete operators. This would be in keeping with the CRUD paradigm. Creating the interface was standard fare, as was moving the database parts of the WhitestarMusicLibrary to a new class DB40MusicDatabase.

Another item I noticed was missing from the design stage was exceptions so I created 2 exceptions: MediaNotFound and UnsupportedMedia. The class structure is now:

In addition to this a general search method has been added to the Database interface. This search method takes a predicate WhitestarMedia instead of using method overloading e.g search(String track), search(Date year) etc. The only issue this presents if one wanted to search for a date range. However at this stage I am trying to focus on structure (SOLID) and not creating functionality.

 

 

Implementation – It is easy to have ideas but practicing them can be hard.

Constantine Stanislavski once wrote:

It is easy to have ideas in art but it can be hard to put them into practice.

I have long learnt that this applies to everything, not just art. In recent posts I have reviewed a project, WhitestarMediaLibrary, using SOLID and other design principles. I have got to the stage where I feel I should start writing some code and altering the project so it reflects the new structure.

However, there are issues with the implementation and over the next couple of posts I will cover these.  The first of these problems is in regard to the libraries. In previous posts I discussed using a Properties object that contains the class to be injected and any configuration that is required for the object is then fed the same properties object. What I did not decide in previous posts is which class is responsible for instantiating the library.  I also added two custom exception classes if an error occurs when instantiating the library. The resulting class diagram is below:

 

 

I am now thinking, was this the best way to do this? Could I have used a factory method where a class takes the config object instantiates the library and the factory feeds the library the properties file? This would be more in keeping with Single Responsibility Principle. Now the WhitestarLibrary class retrieves libraries but delegates the creation and initialisation to a factory class e.g. LibraryFactory. The resulting structure is now: