Thursday, April 28, 2011

Good Values Pay Off

(Cross post from IRefactor)

To simply put, we have been acquired!
For the past couple of years we have been working hard to enhance our technological solutions, the de facto: scalable, near exact match, visual search engine. Using this visual search engine as a backbone, allowed us to envision and to develop many of our great products.

Not surprisingly, PicScout not only became the market leader in the image copyright solutions, but also provided unique solutions in the terms of crediting every image on the web.

On a personal note; Serving as a VP R&D during this time, building the architecture, driving the technology and working with the greatest team ever - was remarkable and enjoyable. I am happy to be a part of a great team where professionalism matters and which employs many of the Software Craftsmanship values, I so deeply encourage!

A few moments ago, we have officially announced that we (PicScout) have been acquired by Getty
Images
.

I am sure that our unique technological value will continue to benefit to the world of images.

Thursday, February 24, 2011

One small step for a man, one giant leap for mankind


(Cross post from IRefactor)
Lately I had a lot of thoughts about how to introduce a change within an organization.
For a while now I am giving some talks about leading a software development team, focusing each time on a different facet, such as: Quality, Architecture, Recruitment Process and etc...
(I have combined some of those thoughts into a short lecture, which I entitled Fostering Software Craftsmanship (Building Successful Teams) and it is given as a part of the IL Tech Talks).
But here is a phenomena, I encounter each time.
Most of the audience understand the importance of the concepts, but really struggle with introducing them to their organization.

Martin Deutsch
  • Writing Unit Tests will lengthen my development time!
  • How much time should I "reserve" for doing Refactoring?
  • How do I explain to my manager that Refactoring is important?
  • Code Reviews are difficult, because people have different tastes!
  • Code Reviews are taking too long! We will not be given the time to do it, ever!
  • Continuous Integration will take a few weeks to accomplish. I won't be given any time to do it in the near feature!
  • Those concepts are great, but the time to do them should be allocated by the management.
  • ...
I am sure we all heard those sayings. Heck, we even have given those once! (No... Not really :) )
Though I have a lot to say and comment about each and every one of the above statements - there are some broader aspects I must cover first.
We all hate changes, don't we? Doing Unit Tests, investing our time in Refactoring (or Builds) - those are just an extra activities that we have never done (or done correctly).
One should really invest his own time in order to polish those activities and doing so is just recognizing that we need to change. As humans, we find it extremely difficult to accept changes.
Jack Welch understood it perfectly, but nevertheless he succeeded to introduce a lot of changes to GE. Jack Welch found the change to be exciting, daring and imaginative.
GE success can be rooted to its ability to change and to change fast.
However, applying change in the simplest way doesn't work.
We cannot accept the change all at once. Evolution takes its time; Therefore, in order to accept the change we also should adapt one step after another.
(It's really like Refactoring Steps: One small change at a time, compile and test).
Sometimes it takes years to introduce all those concepts to your teams:
Start by a simple task, like code conventions or source control layout, then teach unit tests and invest in teaching code smells, continue by applying rigorCode Reviews and etc... and etc...
Doing everything in one big bang just doesn't work. I have seen several teams that got introduced to Scrum or TDD in a matter of weeks. Those are big changes to somebody who didn't evolve to the "right" point. Needless to say that the failures that those teams experienced were so discouraging that till now some of those Software Engineers hate TDD and claim that Unit Tests (even not TDD) are plain waste of their time.
You shouldn't be surprised that applying all the great concepts to your organization will take years.
One step after another; One small step for a Software Engineer, one giant leap for an Organization.

Close-up of Astronaut?s Foot on Lunar Surface - NASA Images

Monday, January 3, 2011

Code Smells

(Cross post from IRefactor)

If you like practicing in identifying code smells, then you can find below a short class called
TimerManager.

public class TimerManager
{
    public delegate void TimerCallback(object data);
    private static readonly object _sync = new object();
    private readonly Dictionary<int, Timer> timers = new Dictionary<int, Timer>();
    private readonly Dictionary<int, TimerCallback> callbacks = new Dictionary<int, TimerCallback>();

     public void SetTimeout(TimerCallback timerCallback, int snooze)
     {
            var timer = new Timer(snooze);
            lock (_sync)
            {
               timers.Add(timer.GetHashCode(), timer);
               callbacks.Add(timer.GetHashCode(),
               x =>
               {
                  lock (_sync)
                  {
                     var t = timers[x.GetHashCode()];
                     t.Stop();
                     t.Close();
                     timers.Remove(x.GetHashCode());
                     callbacks.Remove(x.GetHashCode());
                  }
                  // invoke function provided by caller
                  timerCallback(null);
                });
            }
            timer.Elapsed += timer_Elapsed;
            timer.AutoReset = false;
            timer.Start();
      }
}

Here are some Code Smells, that can be identified:
  • TimerManager (a weak smell) - Everything that is called a manager alerts me a great deal.
    Usually managers are God objects, have low cohesion and high coupling and violating too much
    OO principles...
    It doesn't necessary mean that the same happens here, but it should be noted and then verified in the code.
  • TimerCallback should be generic or restricted by a Data Type.
    It's better to specialize the TimerCallback with more concrete type parameter. Having said
    that, it might be that the author wanted to use the built in TimerCallback in .NET.
    In such a case the declaration is redundant.
  • static _sync object.
    The class has only instance members, besides that _sync object; Will it make sense to do the following?
TimerManager timerManager1 = new TimerManager();
timerManager1.SetTimeout(...);
... 
TimerManager timerManager2 = new TimerManager();
// here timerManager2 will block on the same object as the timerManager1
timerManager2.SetTimeout(...);
  • Both Dictionaries are just plain procedural programming definitions; It is the same like having 2 arrays aligned by their indexes as both the dictionaries will be accessed by the same index (an int). Behold, as one mistake (accessing the wrong index) can lead this code to an undefined behavior.
  • GetHashCode shouldn't be used to identify uniquely an object.
    Let me repeat again... GetHashCode mustn't be used to identify uniquely an object...
    GetHashCode's implementation is needed for collections (and Equals, see below).
    Sometimes the same result of the hash code will put two different objects in the same hush bucket - which means those are not unique identifiers!!!
  • Once implementing GetHashCode, the Equals method should be implemented as well.
  • Consider the follwoing code:
t.Stop();
t.Close();
  • One should be familiar with the tools at the hand.
    Close() does what Stop() do and more... Calling Stop() and then Close() just makes the method a little bit longer without any effect.
    When in doubt, consider to take a few moments just to review the provided API.
  • Redundant Remark.
    The remark repeats the code, doesn't bring anything informative enough and affects the length of the method.
    It can be safely removed.
// invoke function provided by caller
timerCallback(null);
  • Thanks to Aviv the following is of course another smell: passing null to the callback function (i.e. declaring the parameter in the callback and eventually passing null).
timerCallback(null);

Saturday, October 2, 2010

The Importance of Mentorship

(Cross post from IRefactor)

It's hard to become a professional. It's even harder to become a professional Software Engineer.
Last week, during a small management conference I bumped into an old friend of mine, who I didn't see for a couple of years. Being a leader of a software engineering group, he was frustrated and worried:
"I have a group of 20 people, working hard to meet harsh deadlines. The project has just started, but most of the software engineers are already not pleased. There are junior developers that consider themselves as senior developers, there are senior developers that consider themselves as team leaders and there is no uniform professional knowledge. All of these are affecting the product's quality and really hurting our work."
Although there are many ways to shape cohesive and gelled teams, there is one important notion that just lately received its attention: Mentoring.

The professional life of a Software Engineer is rather sporadic.
Most of us are drifted with the stream. The lifecycle is pretty much the same; You find a job, you receive features or requirements to implement, you code, you debug and then you move on.

But, have you ever felt that if mentored by a true professional you would succeed more?
You would know deeper. You would decide better. You would have better choices of your career paths.
I reckon that most of us did feel the same...

Moreover, good mentors, not only promote individuals, but usually create around themselves a great environment (and therefore great teams).
Excellence drives excellence and professional excellence usually directs all the team members to shared ownership, partnership and targets.

However, it is hard to find a true professional and it is even harder to find a true professional that knows how to mentor.

There are a lot of parameters that shape a mentor, but no doubt the first virtue will be experience: Experience of successes, experience of failures and experience of crisis-es.
Those are true Software Craftsman, that spent their days and nights in polishing their skills.
And being masters in building and managing software, they can teach you a lot: How to write good requirements, how to architect, how to design, how to produce clean code, how to build software, how to ship software, how to manage technological teams, how to receive better technological decisions and etc... and etc...

There is much more to say about mentoring. I will dedicate a few posts in the future, to describe what are, in my opinion, the virtues and the ways to do it well.
However in the meanwhile, I would like to take up the glove I have thrown in this post... On our next Software Craftsmanship Meetup, I will enroll a Mentorship program.

A Mentorship program will allow Mentees to find an appropriate Mentors.
The mentorship period will be for at least 4 months (IMHO, it should be more than 4 months, but let's start with that period).
During the mentorship period both the Mentees and the Mentors are committed to each other.
They will meet for at least 2 hours each week in order to:
  • Provide Technical Guidance and Feedback.
  • Provide Reading Sources.
  • Review and Build Carree Paths.
  • Hold Code Reads, Code Reviews and Pairing.
  • Do Architecture and Design Reviews.
  • Review Management Decisions and Advise Management Dilemmas
  • Etc. . .

If you are not coming to the 4th Software Craftsmanship meetup, but still want to participate you are most welcome to contact me via the blog.

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.