Refactoring – Single Responsibility Principle (SRP) applied to JobMan

The MainJob Class stores the data regarding a Job but also issues subjob numbers. Similarly the JobDatabase issues the MainJob Numbers. The Job numbers follow a particular scheme. So the scheme could be injected into the JobDatabase removing the responsibility from the MainJob class.  This will also impact on JobComparator class which will also have to move to a dependency injection.

The trick will be ensuring the proper comparator with the numbering scheme, or at least instances where the comparitor uses the numbering scheme. The JobNumberingInterface could specify its own comparitor via injection with the method: public String getComparator().

Refactoring Old Code – Divide and Conquer

For refactoring the code in old project Jobman. I have finally finished my first phase of refactoring the. However instead of refactoring all of the code. I chose to concentrate on the relatively small section of the code base relating not GUI related. I have done this with the intention of putting all the GUI code into as seperate project – I.E Desktop JobManager. I then intend to create a restful project that then can be used via multiple platforms. For anyone who is interested I have created a Gitlab repository at:

https://gitlab.com/raw_org/jobman.git

Before I start creating other platform I need to finish refactoring the code. This is to ensure it is now following the SOLID and other Design Principles. As of today, this is how the JobManger minus the GUI elements currently looks like?

In the next post I will start with refatoring to SOLID principles.

 

Refactor… Refactor This!

One of my teachers at school once told me. When writing an essay leave it for a day after it is complete and you will see it in a whole new light. With this in mind I thought as a refactoring practice I would refactor the code my first Big Java project, JobMan. From previous postings I detailed that I wrote with not much thought on design and described it as spaghetti code.

However, nothing could have prepared me for the full horror that is the codebase for Jobman. Firstly I created tried to create tests for the main class. This was a big mistake because the code was littered with Jdialog boxes. There is no easy way setup tests without refactoring. Do I really want to spend time creating tests that will break the moment the code is refactored.

If you want to see what I mean I have setup a GitLab or download from sourceforge.

 

WhitestarMediaProject available on Gitlab and added resources.

After much work I have put the WhitestarMediaProject on Gitlab. The url of the repository is:

https://gitlab.com/raw_org/WhitestarMediaLibrary.git

This was a useful exercise in modifying an existing project so it is better designed, uses testing and is easier to expand or utilise.  I have recently been practicing TDD and with me being a big ‘Getting things Done’ advocate I have added to testing checklist the RGR cycle. Many thanks to Uncle bob (http://blog.cleancoder.com/uncle-bob/2014/12/17/TheCyclesOfTDD.html) and Jpassion for providing the inspiration of the checklist. Speaking of inspiration I have included a distilled GTD checklist in the resources section.

 

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:

 

 

Further Design Priniciples

As part of refining the design of the WhiteStarMediaLibrary, what other design principles could I apply?

 

Encapsulate what changes

This is defined by JPassion.com as:

“Encapsulate the code that is expected to change in the future.”

So when looking at the WhitestarMediaLibrary the areas I would expect to change are the MediaType, CatagoryOption and the Library implementation classes. I have already used an interface for the library class with this very thought in mind. However the MediaType and CatagoryOption classes are Enums. How would I add Book to the MediaType or add a new music genre e.g. grime? Enums, by their very nature, are static and so it would be impossible to do this at run time unless we used dependency injection. But this is then reliant on the developer to add this every time a new genre comes out. The original reason I picked Enums was for their speed, how I could use them to group subgenres together and how I could have consistency in music types between differing libraries. This will require further thought.

Don’t Repeat Yourself (DRY)

It is recommended that abstraction is used to group common functionality in one place. This design already incorporates this in the abstraction of whitestarmedia. From looking at the design I cannot see at this stage what other functionality is to be abstracted.  I do feel there should be more than just the whitestarmedia. Perhaps when I start to finish the implementation further examples will come to light.

 

Don’t look for things. Ask for things – Dependency Injection.

When I was looking at ‘Encapsulating what changes’ in a previous post, I discussed the MediaType and CategoryOptions and how changes with introductions of new media types or music genres could be included without having to use dependency injection.  However, if I chose dependency injection and used this to add new genres for example, this could cause problems.

Imagine a new music genre is created called ‘crocky’ (county rocking yelping), a new module is written which is used to replace the existing CategoryOptions class. However ‘Crocky’ never takes off and it is removed from the next version of CategoryOptions. You can guarantee that somewhere, someone will be a Crocky fan and find their favourite media player can no longer find their tracks because the library will not be able to understand the category retrieved by the whitestarlibrary.  However there could be an abstract method for when media is retrieved that checks the category type is valid. Then it is up to the library implementation how this is handled. Until I can think of a better solution the CategoryOptions shall use dependency injection.

Dependency injection is also vaunted as an aid to testing. I will therefore have to return to this topic once I move on to the’ Jpassion JUnit, Mocking, TDD and refactoring’ course. For next post I will concentrate on design patterns.

There is a good article regarding dependency injection at the codewhisperer’s blog which can be found here:

http://blog.thecodewhisperer.com/permalink/keep-dependency-injection-simple

 

 

Job Manager – Mind poored out into coding…

Back in 2006 I started a project called JobMan. It was inspired by GTT (Getting Things Done) methodology. Its primary purpose was to facilitate the quick creation of projects that could be broken further and further down into smaller and smaller tasks. As they say in GTT you cannot complete projects, only tasks related to the projects. I also wanted to be able to accurately log the amount of time spent on a project.

The initial UML drawings for the project are given below:

Start-An-Application-with-an-existing-file-that-exists
Start-Application-With-existing-file-that-is-not-found
Stop Watch UseCase
Word-Processor-Use-Case
Creating-A-Job
JobManager Class Diagram 1
JobManager Class diagram 2
JobManager Class Diagram 3
JobManager Use Case Diagram 1
JobManager Use Case Diagram 2

I intended to use other open source projects where possible and did so for a date picker and DB40 for an embedded database. Also at the time I was more intent on learning than creating something that would be used by someone other than myself. Hence, after the initial design, I coded and created features on a whim. I poured my mind out into my coding and tidied it up afterwards, usually when I found a bug.

The project was important in my development as it made me realise that projects need planning to avoid massive restructuring later. I learnt that design patterns could solve the problems I previously procrastinated over. Also my UI design needed work, which prompted me to buy ‘Filthy Rich Clients’ by Chet Haase and Romain Guy,although some of their work is now redundant given the slow rise of JavaFX2.

 

By 2009 I released  the project on sourceforge and then released its  current version in 2016 after spending quite some time using it in my current position. From this usage a number of bugs were fixed and the project optimised, e.g caching, listing subtasks by reference and not by objects.

 

Solid… Solid like a sock!

As continuation of the post Design Patterns and Good Practice post I will review and amend the Whitestarmedialibrary using the SOLID design principles.

Single Responsibility Principle

 

As it has been described to me the Single Responsibility Principle (SRP) is:

There should not be more than one reason for a class/method to change, or a class should always handle single functionality.

Also responsibilities should not be mixed into one class. The benefit being there is less coupling and therefore it is easier to change parts of the system without breaking something. Now do I understand this?  This may not be as silly a question as one might think. When I looked up SRP I found an article at https://www.toptal.com/software/single-responsibility-principle which discusses this directly from an MVC perspective. He argues the application logic is divorced from the controlling class. If I apply this to the WhitestarMediaLibrary the application structure could look like this:

This reduces the role of the WhitestarMediaLibrary to handing out various libraries and opening the initial database or databases. Do I want a situation where 2 or more libraries are interacting with the one database? Do I have a database for each media? Or do I restrict the library to manage one type of media at a time? This seems restrictive given someone may want to play music whilst they game or record TV whilst watching a film. I will delay this decision further down the line until I have applied the other SOLID principles.

However, I do need to move the database management to another class. This class shall be called WhitestarDBManager.

Open Closed Design Principle

 

Wikipedia states Open Closed Design Principle (OCDP) as “software entities (classes, modules, functions, etc.) should be open for extension, but closed for modification” My first thought was that because this library is an uncompleted project this principle would not apply to the Whitestarmedialibrary. Only then, when the project is compiled and used in another project, would this principle apply.  However, if I declared WhitestarMedia as abstract it will allow for possible dependency inversion later and allow the possibility of new media types to be added later.

 

Liskov Substitution Principle(LSP) Principle

When looking for instances where I have overridden classes, I was unable to find anywhere I violated the super classes features. The only classes where I  could do this were  the WhitestarMedia class and its subclasses. However, the parent class in this instance was created to group common behaviour not as an interface for a subclass to implement.

 

Interface Segregation Principle

This has been stated in Jpassion course as:

 

“Clients should not be forced to implement interfaces they don’t us. Instead of one fat interface many small interfaces are preferred based on groups of methods, each one serving one sub module.”

 

At present there are no formal interfaces used. This however may change when I come to implement playlists.

 

Dependency Inversion Principle

Wikipedia describes this as:

 

  1. High-level modules should not depend on low-level modules. Both should depend on abstractions.
  2. Abstractions should not depend on details. Details should depend on abstractions.

 

As suggested above (in the OCDP section), changing WhitestarMedia to abstract changes the dependency between a library and its media to the abstract WhiteStarMedia and a specific media implementation e.g WhiteStarMedia_Music. However, could this also be applied to libraries themselves? In the above SRP section I moved the functionality of the database and the media management to specific classes. A WhiteStarMediaLibrary Interface could be created so new libraries can be added later. If the interface defined a constructor that required the type of media managed, the main whitestarMediaLibraryClass would could keep the various libraries in a collection instead of specific local variables. It would also allow multiple libraries of the same type to be added e.g. a library for online music service and one for a local service e.g. mp3 collection.  The resulting structure would look be:

This brings me back to another delayed decision, are databases to be single or multiple users? If I want a system where there can be multiple libraries managing media of the same type and some of these are streaming services such as Netflix or Spotify, then we would not be handing a database to them. A better option may be handing over a configuration file when initialising the library. This could be as simple as a properties file that is retrieved from local storage.  The file could be created and configured by the type specific library itself. This is then handed back to the WhitestarMediaLibrary when a close method is invoked. The DBmanager can  be moved to a utility library and then whether the library is shared or not is left to the implementation of the whitestarMediaLibraryInterface.  The resulting structure is reduced to:

All of the classes are then placed into packages e.g. Utility or WhitestarMediaTypes