Know more and connect with me on [Linkedin Profile].

Saturday, November 17, 2018

JHipster Pros and Cons

Jhipster is a web application scaffold that generates backend and frontend code for CRUD applications. You describe entities in JDL format among other options and Jhipster generates backend server and frontend UI for you.

Here are some of the advantages:

  • Fast application code scaffold. In a short time, an application is generated. 
  • Many options exist on the frontend side such as React, and Angular. Vue.js is under development.
  • Also, many options exist on the backend side, such as monolithic or service-oriented architecture. 
  • Backend code is generated in Java code only. There is an in-progress work for Kotlin.
  • It generates automated tests of all kinds, unit, integration, performance and even e2e tests.
  • The code is well organized and uses best practices. You can learn a lot from it.
On the negative side:
  • Jhipster generates tons of code with many libraries that you probably do not know some of them. You can not go ahead and add your business logic and modifications without reading tutorials about these technologies.
  • Somewhat overuse of component, libraries, and frameworks that make your initial greenfield project complex from day one. It is like start big, rather than start small, but that was your choice from the beginning!
Alternatives:
  • JHipster is based on Yeomen. There are so many other scaffold generators, look at http://yeoman.io/generators/
  • You can create your own Yemon generator to generate the code that is just right for you.

Refactoring and Git Developer Story

I refactored a critical long class that has very long member functions. My main intention was to:

  • Extract many smaller member functions to enhance class modifiability.
  • By breaking the long member functions into smaller ones, it will be much easier to understand what the class is really doing.
  • By having many smaller member functions, we have the chance to discover if the class is not cohesive. We may then consider extracting a new class. This will be a good news as the class is already too long.
  • Enhance the memory footprint for the main class API. 

I did many commits, each with a logical and small refactoring step. When the code is sent for code review, the reviewer complained of time wasted reviewing all these commits. He asked, for next time, can you make it one big commit?

No, we can not, and here is why it is better to have fine-grained commits.

If you are doing a real code review that you have to understand each change to ensure it meets quality checks, ensure there is no bug injected and evaluate side effects and to propose QA test cases to cover them, then one small logical commit is much easier and accurate to review. It will take time, but it is worth the time.

For the developer himself, I find it very crucial to commit changes bit by bit, ensuring logical and small change is committed and a good comment is written. It is like a self-review and organized way even before code review.

If still, the reviewer needs to see all changes in one shot, he can easily check out the old class and the final edited class and used any diff tool to see all differences at once.

Happy coding!

Tuesday, July 31, 2018

Lesson Learned: Code Regression


Giving advice is not very effective most of the time. Sharing real experiences and trouble and how it is handled is more effective. The best is whoever learns from others mistakes. So I am sharing here a story where I and my team introduced a bug and it was leaked to the production environment. I know most of the advice here when I was consulting people in Agile but when the situation changed, I did that same mistake. The shame was painful and hence the learning was intense. Experience is where you really learn, not advice from ivory towers.

I am sharing here hoping that it will be useful for myself and for others.

Look at this code fragment that is a part of timezone based message broadcast.
    private String timezoneMetadataId = "23467fe6523310";
       
       ...

     //Check Metadata for Timezone
    List subTimezoneValue = sub.getSubscriberMetaData().get(timezoneMetadataId);
    for (String timezone : timezones.split("[;,]")) {
        for (MetadataValue mv : subTimezoneValue) {
            //Do some processing here

        }
    }


The question here: Is timezoneMetadataId acceptable to be null? In our case, it must not be null. The initialization above ensures it is not null.

Until now, everything is ok, but the hardcoded timezoneMetadataId is not recommended especially as it is repeated in many places in the source code.

We wanted to change this hardcoded id and place it inside the configuration file. As we use Guice dependency injection, we changed the above code to be:


    @Named(value = "mongo.wmprod.subscriber.metadata.timezoneId")
    private String timezoneMetadataId;


And here we made the mistake of forgetting the @Inject annotations which meant the @Named annotation has no effect and the variable, timezoneMetadataId is initiated to null. Sadly the above for loop code will not raise any errors as getting the value of key null is an empty list and looping over the empty list is ok.

This is a code regression. The changes that intend to make the code better ended injected a critical bug.

We have a Code Review process, the reviewer did not notice the mistake. Maybe because it was buried into a broad set of changed lines. 

QA also did their own testing, they did not run the timezone based broadcast test case. 

Here I am sharing lessons learned to avoid such regressions in the future:


Lessons Learned for Programming:

You are changing the code. Do not be fooled by the simplicity of your changes or assume safety. Follow proven programming practices all the time.

1) Verify value is not null

All configuration items must be verified. See below:

import com.google.common.base.Preconditions;
Preconditions.checkNotNull(timezoneMetadataId);

You can also verify the size of the key or the validity of the key. Fail Fast is your best friend.


2) Add log statement that shows the id.

logger.info("Sending broadcast with timezone metadata id: {}", timezoneMetadataId);

If the value is invalid in some way, you can check it in the log files.


3) Write a unit test that ensures failure in case of null or invalid timezone metadata.

Actually, this will ensure that your preconditions are really working. It is indeed easy to make mistakes. Ensuring your preconditions and verifications are working is essential. If the project has no unit tests at all, create a task to create your first unit test immediately. 


4) Write integration tests. 

In this project codebase, there was no integration testing framework in effect. If you join such team, create a task to create your first integration test in your first employment week.

5) Use Constructor Injection
See https://github.com/google/guice/wiki/Injections

Lessons Learned for Code Review:


We did not have a written checklist for code review. This is a mistake. Even if you join a team that does not have a checklist, do not wait until you have team experiences to create your own list. Conduct a meeting and start with a simple checklist immediately. With each Story or Bug Code Review, the checklist should be filled and attached to the Story or Bug. The checklist template should be updated frequently.

We took Code Review seriously but with no structure. The above ideas are to make it serious process with explicit checks and responsibility. 

Lessons Learned for QA:

We kept assuring the safety of changes to Project Leader and QA. This is really a mistake. Fears should be reduced by adding more test cases not by getting developers assurance about the safety of the changes.

I find it necessary to list all changed functionality to QA. It is even better for QA to have access to changes themselves. Once QA read source code nouns such as variables names, functions names, and class names, they can associate it to UI functionality and create the corresponding test cases.


In the end, I hope my mistakes and lessons learned will be useful.


Wednesday, July 18, 2018

Refactoring Tools

I used IntelliJ to do some package renaming refactoring. I was confident that it was safe to do so, however, other parts of the code had changed when they should not have. Later on, I discovered the problem and fixed it. I used to avoid find/replace tools or grepping because it is dangerous as you can easily change parts that should not be changed. 

I learned to never trust any tool regardless of whether it is smart or not. In complex code projects,
you have to be careful all the time and avoid being deceived by automated smart refactoring tools. I learned that using unsafe find/replace carefully is better than using smart refactoring tools carelessly. 

Monday, January 29, 2018

Technical Codebase Visibility

In the last post, I wrote about the problems that happen because of the lack of technical codebase management.
First, you can not do anything without knowing your status right now as well as where you are heading. Putting your goals in mind, then you are fully equipped to do the right things and monitor your numbers to make sure you are on the right track.
How often should you monitor your metrics? In my opinion, given the fast paced age we all live in, we should measure weekly.
In selecting those metrics, I put into mind the following criteria:
  • No single metric will be sufficient. We should select a portfolio of metrics that shows the complete picture to avoid the dilemma of fixing something at the expense of something else.
  • Every metric should be simple to understand. Even if it is not perfect on its own.
  • Every metric should be easy to measure using open source tools.
  • Metrics should be measured on team level. So integration using Continuous Integration server such as Jenkins is necessary.
Here are some proposed metrics:
  • Code size measures. Seems trivial, but I can not imagine working on a codebase that I am unaware of its size. We can count things, such as Lines of code, classes, HTML templates, etc.
  • Static code analysis using SonarQube. It is nowadays trivial to install Sonarqube using Docker and analyse your code. SonarQube provides coding-standards checks and code-smells checks. I like to describe that process as a learning process as it enables developers to learn the best coding techniques and patterns. If your developers do not learn that, they will simply repeat their mistakes, even with a green field codebase.
  • Duplicate code analysis. Sonarqube is providing it.
  • Count of automated test case. If you have many, congratulations. Now, you can move one step further and check the percentage of unit tests compared to other test types. You can do deeper analysis and ask yourself questions, such as: Is this kind of test sufficient?, What kind of manual tests should be automated? Are test cases really helping your team? How to make it more useful?
  • Code coverage: The count of test cases is rarely useful in itself. 1000 test cases may be good for a small system, but for sure insufficient for a bigger system. This is why the percentage of covered lines of code with tests is important.
  • Manual test case count. Which areas of the system are covered and which are not. Where should you have more manual tests written?
  • Bugs analysis. The target of this analysis is to answer the following kind of questions. Which parts of the system generates the most bugs? Who is the developer who injects the most bugs?
  • Build a Refactoring Backlog. Tools are getting better everyday, but it is not perfect and it will never be. Developers already know system problems such as, which parts of the system are the most complex, which legacy component should be replaced, etc. Write all these tasks in a refactoring backlog, with attributes such as, estimated time, complexity and risk of introducing new bugs.
Now, you should have visibility on where your codebase is standing and the trend of each metric. Now the question is: how to plan and track the effort required to change the codebase trend? For example, if code coverage is only 9%. How can you plan and track the effort required to increase it. That will be the topic of next post.
Finally, thanks to Nour-Eldeen, my son, for reviewing this article.

Sunday, January 28, 2018

Lack of Technical Management

I worked on many projects and found a repeating anti-pattern that is happening to the codebase, which is the lack of technical management. Here, I am sharing my thoughts about it.

The problem definition:
  • The team has no understanding of the size of the product and its complexity.
  • There is a lack of metrics and understanding of source code quality, that includes the source code itself as well as its design and architecture.
  • There are no defined coding standards, as well as design and architectural standards. 
  • Lack of metrics regarding automated testing and code coverage. Sometimes, there is a complete lack of test strategy. 
  • No measures to the amount of codebase changes and how the codebase is affected. 
  • No statistical understanding to the quality of the product from customer’s perspectives, e.g. bugs. 
In general we need to know the baseline now, as well as, on weekly basis how the codebase is changing overtime. For example, is code quality deteriorating or improving? Is complexity increasing? At what pace? What about code modifiability?

In general I understand why project management is done properly, most of the time. Usually, because of the customer pushing to get things done, and our understanding that, if we are not able to satisfy our customers, our competitors will. But, what if you mismanage the product technically and focused solely on feature delivery? In the short term, no one will notice, but as time passes, here is what happens all the time.
  1. The code base becomes harder and harder to understand. 
  2. Code modifications becomes time consuming.
  3. Regression bugs increase. 
  4. The fear of changing code increases rapidly. Copy/paste strategies flourish and lead to a larger codebase and a lot of duplicate code. And, then new inconsistency bugs appear. 
  5. Codebase hate becomes widespread. Everyone is cursing the product and nagging to start something cleaner and better structured. Sometimes to use new frameworks or even a new language altogether. Guess what, we start a green field project, and once the codebase grows, and with the continual of “lack of disciplined technical management”, the whole cycle repeats. Just go up to item number one and read again. 

I will share how to resolve this in upcoming posts.