Thursday, September 30, 2010

Effective Code Review - A Presentation

(Cross post from IRefactor)

Here is a short presentation I did a while ago on a Code Review process.

I am sure, there are a lot of different ways to do code review, but the following served me well in the past.

One important note to bare in mind is that a Code Review cannot take more than a few hours.
People get tired easily; It's difficult to read somebody else's logic (code) and even more: to understand and to pinpoint the problems.

Therefore the Code Review process should be effective and highly cohesive as much as possible; I like the following metrics:
  • Summary: Ask a Software Engineer you are reviewing to explain in bullets what are the purposes of
    the application, the module or the feature you are going to review.
  • Top-Down: Require to review high level components; High Level Design (Class Diagram, Component
    Diagram, Some other Sketch and etc...)
    Understand in general what are the main components / classes in the solution.
  • Focus: Ask the Software Engineer to point you towards the most problematic sections of the code.
    He is the one who knows best what was hard to design or to implement.
    Ask him to explain what was the problem and how he tried to resolve it.
  • Bottom-Up: Now start from the code you focused on (You can also review the Unit Tests in order to
    understand better the code's behavior).
  • Read and Communicate: Take an ownership on the keyboard and observe the code. Try to see whether it is easy to understand the code and to navigate through it.
    During your drive you should communicate loudly what are you doing; Where you are going next (e.g. "I'm opening class X"), what you think of a piece of code you are reviewing and etc... You should ask questions in order to understand why it was implemented the way it was and also to verify that other alternatives were considered by the Software Engineer.
    You should spot the following:

    • Bad naming (classes, methods, properties and etc...)
    • Code Smells
    • Design Violations (SOLID, GoF and etc...)
    • Architectural Flows
    • And many others (I won't discuss here currently)...
  • Your mutual notes should be written by the reviewed Software Engineer (navigator) and fixed later on. Don't waste your precious time to tackle immediately the problems.

Tuesday, September 28, 2010

A Round Table

(Cross post from IRefactor)

Here is an interesting thing about Partnership and Shared Ownership.
Once creating such an atmosphere within your team (company) you are on the road to success.

Everybody works together in order to accomplish the shared targets: Product Managers, Engineering, QA, Market Managers and etc...
Usually, small teams and small companies are characterized by the "Partnership and Shared Ownership" DNA.
But when they grow, rest assure that slowly but surely the Partnership and the Shared Ownership notion will dissipate.

Once reaching ~150 people it is almost impossible to share the same values as you did before. (And in my humble opinion, it happens much much before 150)
Ah, but then a miracle happens...
Some "smart" manager gets a wonderful idea how to revive the sense of Partnership and Shared Ownership.
He suggests to have a "round table" with the President\CEO\Group Manager.

Indeed, the President cannot meet each and every employee, but he can share a sense of equality as the King Arthur did with his knights.

 Let's have a bunch of employees sitting together, discussing their important values, tasks and views with his majesty.
Alas, I have been there; Allow me, then, to elaborate regarding what really happens around this table:
  • Either most of the employees are not familiar with each other or are not familiar enough in order to speak from the heart.
  • Therefore, most of the time, the employees will introduce themselves and their projects/tasks without touching the really painful issues.
  • The rest of the time, the President will summarize his activities and his "successful decisions and directions".
  • And then... Well, then the meeting will be over. (But at least the President will be happy. After all, such a "convenient" audience is very hard to find...)
There are only two recipes to succeed in creating the Partnership and Shared Ownership atmosphere.

Involve & Trust.

You involve your employees, they involve their employees and so on... and so on... You spend your time explaining the roadmap, emphasizing values, motivating and also hearing what your people have to say. You verify that you follow what you preach; You incorporate your employees suggestions and opinions. You also give an attribution as your employees should be acknowledged if they have provided a good idea. And you provide a "safety net". Your employees should know that they have a strong figure standing behind them; You don't blame, you don't share your disappointments - you support and foster.

But sometimes, the round table idea sneaks into the small companies.

When it happens, it signals that the problem is even worse; If a manager feels that there is a need to have a round table in a small company, it means that he doesn't have the time to bestow the values on his employees. His hope is to do the minimal effort, bringing everybody in "one take" to the table, in order to "motivate". Though the employees will feel more comfortable with each other in a small company, it just emphasizes the miscommunication and the mistrust that is going on. Clearly, this is not a best management...

Dear manager: Involve & Trust... And do it personally!

Monday, September 27, 2010

Software Craftsmanship - Meeting 3

Our 3-id Software Craftsmanship meetup was as usual divided into 2 parts.

During the first part - Lior Friedman gave us an interesting lecture on Context Based Design, DRY, KISS and SOLID.

Lior also demonstrated how to take a legacy code and apply unit tests on it.

Below you can find the lecture:

During the second part - we had a Coding Dojo.

Some of us implemented the Lychrel Numbers Detector (in Java, JavaScript and C#), while others continued with Lior's example and introduced unit tests in order to refactor the code.

My special thanks to Tzipi and CheckPoint for providing the space.