Sunday 31 May 2015

Hiring: Technical phone screening


Time has arrived to turn to another of my pet subjects: hiring. As any other hiring manager, I went through a long and winding road with varying results, which yielded a few practices, tips and tricks.

As they say at the end of a flight - I realise that you have a wide choice of other hiring guides and recommendations. The one difference here is that I'll be focusing as much on the end result as on how the process was improved on the way.


Why do phone interviews?

It's best to start from the beginning, and in this specific case, it would be the technical phone screening (I'm nimbly sidestepping the various HR phases and the actual procurement of CVs). Now, for most people this stage is taken for granted, but only a decade ago I did not belong to this enlightened group, and scheduled face-to-face meetings entirely based on resumes.

It took a couple dozens of disappointing interviews to realise two recurring points:
  • People can and will tailor keywords to the job spec.
  • Our and candidate's definition of being proficient in something may differ.

Now, before becoming too cynical: very often this misconception does not have any special intent on candidate's part. When they claim that they know C++ they might refer to writing university assignments twelve years ago, and using it in their second workplace eight years back. As I'm looking for a senior engineer, my perception of a C++ skillset is someone who can have a meaningful conversation with an expert and be aware of the latest standard.

(Of course, C++ is just an example - this happened to me with any number of skills, such as Python, Linux, networking, cryptography and so on and so forth)

Both of us are right in our own way, and it's very rare that someone would lie outright on their resume. The problem is that many applicants don't know what they don't know.




Coming to the main point: face-to-face interviews are expensive for the employer, but even more so for the candidate, who has to travel, potentially take a day off at their current job, and spend time on preparation.
Hence, inviting someone without knowing for a fact that they have a decent chance of getting an offer is simply impolite. And, knowing that based on CV alone is impossible.


What phone interviews include?

To understand better what phone screening should consist of, we need to step back and ask ourselves: what do we want to get out of this?

The answer is simple: we want to make sure that the candidate has a reasonable chance of making it through. In other words, if they show up in our office and we realise very quickly that they are not the right fit, then the screening has failed.

Extrapolating this to software engineers (I'll cover QA in a separate post), we need to tick off these points:

  • Can they code?
  • Is there a large discrepancy between what they think they know and what they actually know? 
  • Can we work together, i.e. is there a communication problem?

This is not a huge list, and 95% of the time a candidate that passes these would never be a straightforward rejection later on.

Can they code?


This is the main one. Developers are paid for developing. If they can't do that, then there's no point going much further.

It all sounds painfully obvious, but back in the day I wasn't asked to code even once before being invited over. On the other side of the fence, when we were omitting coding from our phone screening, we were getting candidates who knew theory (or scored a lucky match) but could not put it to practice.

To avoid this kind of situation, some people prefer sending assignments and asking to do those offline ahead of time. My issue with this approach is two-fold:
  1. This method doesn't shed much light on how productive a candidate is; they might have written the assignment in 15 minutes or 15 hours.
  2. If an offline assignment is big, then it's unfair: we ask for substantial time investment before we know there is chance of success. If it is small, then it gives opportunities for StackOverflow- or old-uni-friend-assisted answers, and does not give enough input.
Hence, my preference has been asking to code on a shared document during the call. Naturally, this also has its detriments; a person might freeze under pressure, and logistics become a bit more involved - we need to have a computer at both ends, and there is a slightly uncomfortable wait period, while the candidate is doing their best.

However, my personal experience showed that this method simply works. There was no single case where someone did coding well, and turned out to be an absolute no-go later on. And, vice versa, on a few occasions where I went against my better judgement and invited in people with shaky coding preliminaries, it ended in (figurative) tears.

The coding question is usually very simple, and something that anyone remotely proficient in programming can do within 5 minutes.


This could be reversing a string, determining whether a given list is a palindrome, or find the count of distinct characters. 
(NB: These are not the questions I'm asking today in case you're applying)

For a while, even the infamous FizzBuzz was filtering out a decent subset of candidates, until I decided to do add variety to the proceedings, and raise the bar a tiny bit.

Another benefit of the coding exercise is that it serves as a trampoline to the technical questions. If the code contains an odd construct or algorithm choice (e.g. using a temporary string for reversing), it gives an opportunity to probe theory.


Is there a CV/skillset discrepancy?

First, we need to explore the skills that matter to the job. If, let's say, we do development in Java and the interviewee has a strong Java presence in the CV, then the topic naturally merits a few questions. The trick here is to find questions which are not plainly obvious (i.e. demonstrate experience rather than preparation), and that will give a good insight into candidate's proficiency.

For example, in the Java case, I might ask how its garbage collection works. Here, there can be a wide spectrum of answers: from not knowing what this is to a Wikipedia-level answer covering different JVMs. If it feels like the answer is prepared and I simply played a King to candidate's Ace, I might ask an open question which would push them outside of the straight and narrow (e.g. when do you think your knowledge of garbage collection would be useful during day-to-day work?).

In some cases, even skills not entirely relevant to the job may be fair game. If someone has wide malware analysis experience, and I happen to know it too, we might talk about it. This can work both ways: in worst cases it can uncover CV padding, and in best cases it can show that they do have ability to understand subjects in-depth, and negate shortcomings elsewhere.

Note that discrepancy is not always so simple as know vs. don't know. It happened to me that we went very far in the process with candidates who had decent knowledge of the right technology, but (the crucial "but") they were certain that were far better than "decent", and possessed full mastery, which was simply untrue. This would have created a major problem later on: some people can adjust their self assessment, yet some will inevitably plunge in workplace conflicts when their technical superiority comes under fire.
The key point that it's easier (but not easy!) to acquire new skills than change personal attitude.



For this reason, I used to ask candidates to self-rate themselves on a given subject before drilling in. For example, we would have a problem if someone rated themselves as 8/10 on Python, and would not know what a Global Interpreter Lock is. On the other hand, if the self-rating were 2/10, then it wouldn't be as big an issue. Of course, it's yet another question whether 2/10 is good enough for the role, but at least we are on the same page.

With time, I stepped away a little bit from numerical self-assessment to relieve some of the psychological pressure that inevitably comes with phone interviews, and simply started asking how confident a candidate feels about a specific area. Usually, the certainty and the tone of the answer is a good enough indication.

Can we work together?

Unlike coding or technical questions, this one is very ambiguous. After all, is it even possible to figure out personal compatibility in 30 minutes?

My approach has been asking open-ended questions; someone with data storage in their CV might get "what is your opinion of the NoSQL movement? How does it compare to a full RDBMS?". 
Or, if someone has both Java and Python in the CV: what are your thoughts on strongly typed languages as opposed to duck typing?

(Of course, there's also technical aspect to this, as I'll find out on the way whether they know what NoSQL or duck typing are.)



They do not have a right or wrong answer, and most importantly, they ask for opinion rather than facts. The answers will tell whether the person is opinionated (or not), whether they can present and evaluate multiple positions, and consider someone else's viewpoint. To check the latter, sometimes, even if I agree with what comes on the phone, I might play the devil's advocate.

What do phone interviews exclude?

Now, for the usual flip side of the coin: what's to omit? Phone interviews should be kept reasonably short: both as a courtesy to the candidate, and as a mercy to our own calendar. Recall that we want to have a quick pre-filter, and two hours phone calls are anything but quick.

Here's my list:
  • In-depth questions, such as esoteric corners of a language standard, or a full list of flag names for a UNIX command. There's no point really: we need to figure out if a person is good enough to come over, not whether they are an absolute expert in a specific field. 
  • Review of past experience. Of course, previous jobs matter, but mostly as a path of gathering skills that might be of use for us. Hence, it makes more sense to evaluate the end result rather than the means of acquiring it. In face-to-face interview we might do things differently and go over the CV in detail, but there's not much time for it during a call.
  • HR questions, such as career expectations, main strengths etc. Again, time is short, and in any case: more often than not we'll be receiving stock answers that do not help with filtering the candidate. The right time for this will be the face-to-face stage (especially since these questions work far better when you can see each other).


Interview - not interrogation


Last, but not least, remember: interview is for both parties; the ideal time split between questions is two-thirds company, one-third candidate. As many others stated: phone interview is often the first real impression the candidate will have of your company. If he/she is capable and has the right skills (exactly the type you want to hire), it's very likely that they'll have a choice of employers, so this impression matters a lot.

To that purpose, I tend to follow a number of rules:
  • Never give a verdict during the interview, even if it went great, and the person you talked to on the phone is the living amalgamation of everything you're looking for from a software engineer. Why? You never know what might happen tomorrow: maybe the position will be pulled from underneath you, maybe someone from within the company will apply, and maybe you'll show some of the answers to the team, and they'll notice a gap that you've missed. There are few worse experiences than guaranteeing a face-to-face interface and then reneging on that promise. A special case of that is:
  • Never cut the interview short - even if it's evident after ten minutes that the process won't go much further. This immediately gives a negative answer, and thus violates the previous rule. Also, it is just a matter of basic courtesy.
  • Always close off with the potential next steps and response ETA. As per the previous point: this happens even if deep in my heart I know that we will not be meeting. Regardless, I'll explain how face-to-face interview would run should we get there, and by what date they will hear from us.
  • Always allow interruptions and dialogue. It's not just about satisfying curiosity and giving the interviewee full input (although this matters a lot of course!). In many cases the questions you hear will give a far better insight than the ones you ask.
    For example, years ago, during a senior dev hiring round, I was asked about specific JavaScript sandboxing challenges, which happened to be exactly the task we were tackling ourselves. Needless to say it propelled the candidate up the list.
    Note that you can interrupt too, especially if the candidate goes on a tangent or takes time to impress on topics you haven't asked (this happened to me a number of times). 
  • Never lead off with technical questions. I always prefer to ease in by first taking about the company, the role and why we are looking for this particular profile. This makes the process two-sided, and makes the other party less nervous.





  • Don't sound like a broken record. In other words, when talking about the position, don't say the same words in each call. I know - it's hard, especially once we get going and run a couple of calls each day. However, a brief 5 minute preliminary look at the CV always gives a few pointers and removes that conveyor belt feeling.
    For example, rather than saying "and we use Java for our automation framework" , I might say "and, as with your Initech experience five years ago, we use Java for automation. This is why this part of your career will be particularly interesting for us". 
    This all goes back to first impressions: showing that you carefully looked through the CV is a signal of genuine personal interest, and might be the little gesture that will push the candidate in the right direction.


Summary

Good technical phone interviews are the cornerstone of a good hiring strategy. Without them, you either end up wasting your and candidate's time, or rejecting potential matches through CVs alone. Getting them right is hard but rewarding: it consists of laser focus on our pre-filtering points (coding, CV truthfulness, and personal compatibility) and giving the right first impression to the candidate.

Sunday 24 May 2015

Code comments: explaining the 'why'

While I'm on the subject on code reviews. it's hard to get past one of my favourite subjects: code comments.

You might wonder here whether comments justify having a full blog post. After all, it's quite simple: add a line in plain English (or whichever language tickles your fancy at a given moment), and move on to doing real coding.

My main bugbear with review comments is not so much their absence or presence, but rather their purpose: in many cases, they explain the obvious and leave the complicated as an exercise for the reader (to borrow a phrase from one of my university tutors).

Let me dig out an example:
#include <iostream>
#include <string>
 
using namespace std;
 
bool TestPalindrome(string::const_iterator forwardIt, string::const_iterator backIt)
    {
    for (; forwardIt < backIt; ++forwardIt, --backIt)
       if (*forwardIt != *backIt)
         return false;
    
    return true;
    }
 
bool ProcessTestCase()
    {
    string inputStr;
    cin >> inputStr;
 
    auto forwardIt = inputStr.cbegin();
    auto backIt = --inputStr.cend();
    
    for (;forwardIt < backIt; ++forwardIt, --backIt)
       {
       if (TestPalindrome(forwardIt + 1, backIt) || TestPalindrome(forwardIt, backIt - 1))
          return true;
       if (*forwardIt != *backIt)
          return false;
       }
    return true;
    }
 
int main(int argc, char *argv[])
    {
    ios_base::sync_with_stdio(false);
    size_t testCaseCount(0);
    cin >> testCaseCount;
    for (size_t i = 0; i < testCaseCount; ++i)
      {
      if (ProcessTestCase())
         cout << "YES";
      else
         cout << "NO";
      cout << endl;
      }
    return 0;
    } 

Now, at this point, you don't know what this code is for and how it does it. Of course, a bit of reverse engineering is a nice past-time for a lonely afternoon, but let's say that we'd really like to take a shortcut and see some comments.

So, here they are:
/* Determine whether a set of inputs can be converted to 
   a palindrome by removing a character
*/

#include <iostream>
#include <string>
 
using namespace std;
 
bool TestPalindrome(string::const_iterator forwardIt, string::const_iterator backIt)
    {
    for (; forwardIt < backIt; ++forwardIt, --backIt)
    // If character mismatch, return false
       if (*forwardIt != *backIt)
         return false;
    
    // Loop finished, return true
    return true;
    }
 
bool ProcessTestCase()
    {
    string inputStr;
    cin >> inputStr;
 
    // Set iterators to the start and end of string
    auto forwardIt = inputStr.cbegin();
    auto backIt = --inputStr.cend();
    
    for (;forwardIt < backIt; ++forwardIt, --backIt)
       {
       // Check for palindromes
       if (TestPalindrome(forwardIt + 1, backIt) || TestPalindrome(forwardIt, backIt - 1))
          return true;
       // If iterators are not equal, exit the loop
       if (*forwardIt != *backIt)
          return false;
       }
    return true;
    }
 
int main(int argc, char *argv[])
    {
    ios_base::sync_with_stdio(false);
    size_t testCaseCount(0);
    // Read test case count
    cin >> testCaseCount;
    for (size_t i = 0; i < testCaseCount; ++i)
      {
      // Print out result
      if (ProcessTestCase())
         cout << "YES";
      else
         cout << "NO";
      cout << endl;
      }
    return 0;
    }

The top level comment should take us forward - it at least explains what the purpose of the code is. But what about the others? It's hard to argue with their truthfulness, but they are kind of obvious.

Here, I'm approaching the crux of the message: often, people put Mathematician's answers as comments. I.e. the information is correct, it is in the right place, and yet, it does not help whatsoever.
The right comments add information that's absent in the code itself, and in our example, the code already tells that we read the test case count, print out the results, and exit the loop.

Of course, it's easier said than done. Not everyone can just flick a switch, get an out-of-body experience, and know what everyone else doesn't know. My personal approach to that was adding comments in two phases:
  1. While writing, when knowingly adding complex code.
  2. After finishing coding, and before submitting it for review. There's always a natural break between those two when automated tests are doing their worst. This is the right time to take a step back, read the code again, and pick up quirks that seemed natural on the first pass, but do not longer look that way after a few minutes away from the screen.

Here's the same code example with better comments:
/* Determine whether a set of input can be converted to 
   a palindrome by removing a character
*/

#include <iostream>
#include <string>
 
using namespace std;
 
bool TestPalindrome(string::const_iterator forwardIt, string::const_iterator backIt)
    {
    for (; forwardIt < backIt; ++forwardIt, --backIt)
       if (*forwardIt != *backIt)
         return false;
    
    return true;
    }
 
bool ProcessTestCase()
    {
    string inputStr;
    cin >> inputStr;
 
    auto forwardIt = inputStr.cbegin();
    auto backIt = --inputStr.cend();

    // Run two counters: from the start and end of string. 
    // If they point to different characters,
    // check whether removing one of them yields a palindrome. 
    // If not, we cannot convert the input
    // to palindrome by a single character removal
    for (;forwardIt < backIt; ++forwardIt, --backIt)
       {
       if (TestPalindrome(forwardIt + 1, backIt) || TestPalindrome(forwardIt, backIt - 1))
          return true;
       if (*forwardIt != *backIt)
          return false;
       }
    // If inputStr itself is a palindrome, 
    // we can simply remove a character from the middle,
    // so return true here.
    return true;
    }
 
int main(int argc, char *argv[])
    {
    ios_base::sync_with_stdio(false);
    size_t testCaseCount(0);
    cin >> testCaseCount;
    for (size_t i = 0; i < testCaseCount; ++i)
      {
      if (ProcessTestCase())
         cout << "YES";
      else
         cout << "NO";
      cout << endl;
      }
    return 0;
    } 

Comparing the two examples one against the other:
  • No more comments which tell what the line below does, such as "Set iterators to the start and end of string". If the reader knows C++, they won't need it. If the reader does not know C++, then they'll need far more than that.
  • New comment explaining the main algorithm (lines 27-31).
  • Comment highlighting a specific edge condition (lines 39-41).
The last one exemplifies the difference quite well. If someone annotates code as per the first example, they would never consider commenting what return true does. But, the important bit is not what it does, but why it's there, and why it does not return false.

Coming back to the two-phase approach: if I need more than a minute to think of a specific code line, then it is complex, and probably merits a comment. In this specific case, the return true took me a couple of minutes, so it got a comment as its chaperon.
And, to close a circle, let's mention code reviews. If someone needs clarification during review, then nine times of ten the answer should manifest itself within the code.

So, here we are: comments should explain 'why' rather than 'what', and they should be read and written by taking a 3rd person view.

However, it's also possible to take things into the other extreme. If you find that comments overwhelm the content, then it's quite possible that the content itself is the problem.

For example, I could add a comment for the line below:
res = itertools.product([x for x in myList if x in y else calcValue(x)], anotherList)
like this:
# For all values not in the intersection of x and y, recalculate value, and then create a product with another list
res = itertools.product([x for x in myList if x in y else calcValue(x)], anotherList)
or I could just rewrite the code to make it more explicit:
newList = []
for x in myList:
   newValue = x if x in y else calcValue(x)
   newList.append(newValue)
res = itertools.product(newList, anotherList)
The last variant is more readable despite not having a single comment.

Sunday 17 May 2015

Presenting to executives


A while back I talked about presenting status updates to fellow engineers. That talk ended on an ominous phrase:
A customer meeting and an exec presentation have complete different audience and focus. 
The moment has arrived to lift the veil of mystery, but firstly let me throw in a

Disclaimer

I was lucky enough to work for execs who had strong understanding of market and technology, and did not favour personal gain over company success. So, when you visualise an exec throughout this article, banish the image of PHB or Dilbert's egg-headed CEO. Rather, think of someone whose job equally depends on the merit and success of what you are presenting, and who understands the business at least as well as you do.


Different from status updates

The first step in every successful presentation is figuring what the audience cares about, and shaping the material accordingly.

Of course, saying that all leaders have the same motives and interests would be disingenuous. Everyone has their own perspective, and people who have been in the industry a while and risen high up even more so. However, there are a few common questions that usually hover in the air:

  • Does the team know what they are doing? I.e. how easy is it for me to poke holes in the approach? Are they the right guys for the job?
  • Can this ricochet into other departments? Have they missed any dependency on others (e.g. marketing campaign, documentation, supportability)? Is there a wide enough grasp on the project?
  • Will this go/over under budget? Am I happy with the effort and money invested? Is this justified compared to the other projects on the radar?
  • When and how will we monetise this? Is there a sound commercial foothold? Does the team understand the business side of what they are working on? Do they have the right understanding of the market when making project decisions?
  • How does this fit into the company concept and strategy
Again, there may well be more questions involved, but I'm yet to present to an executive who did not care about these. A bit further down, I'll take a sample status update and have a look at how we can pre-answer these points, but beforehand:

What to avoid?

Here is my personal taboo list:

  • Long, 30+ slide presentations. Time is money. For a VIP, time is a lot of money. Most sane people dislike death by PowerPoint (yours truly included), but your bosses can convert dislike into actions. Don't do it.
  • Complex project and dependency charts. The exec is not there to do a thorough review of your plan. Showing a huge chart or complex dependencies is akin to saying: "please do my job for me", which is one of the worst messages that can be conveyed. It's good to be able to show that you're in a full control of the situation, which is why this type of information should be cast into the backup slide dungeon.
  • Low-level requirement breakdown. Same as above, for the same reasons and with the same resolution.
  • Emphasis on technology. In general, technical terms are not out of bounds, especially if the C- on the other side is followed by TO. But, it's important to show that acronyms is not all you care about (see how will we monetise this in the list above).
  • Self promotion. The word "I" is great in general (though I already overused it by this point). Just try and minimise it when presenting. 
  • Internal politics. Even a slight smell of those can trigger an alarm. Recall one of the questions above "Do they know what they are doing?". If they don't work as a team in the first place, then this question goes to a totally different plane with a potential aftermath to follow. 
  • Typos and visual inaccuracies. It's important to show that you treat this occasion seriously. Typos and misspellings might be ok in many situations, but not when you have stakeholders on the line.

What are the signs of a good presentation?

Many questions with quick and acceptable answers usually signal the path to success. Vice versa, if you rattle through slides accompanied by funeral silence, then something is going wrong. In many presentations having an 80/20 split between talk and questions is ok, but not here.

As a rule, I usually strive for at least 50/50 talk-to-listen split when presenting, and if this ratio seems to be far off, try and ask for opinion before jumping to the next slide. Remember, your goal is not to display all the slides, but to address the execs' questions. One of the first signs that slides do not match the agenda is silence - and one of the first actions to fix that is to discover said agenda by asking questions.


This also implies another point: interruptions are good. If you get stopped mid-phrase, great. This is not a poetry recital, and the job at hand is not getting through the PPT, but instilling confidence.




Sample presentation

Let's pull out my one slider from a previous blog post. This is what fellow engineers would see about our achievements from the past fortnight:



And, this is exactly what I'm not going to show to an exec. Rather we need to explain why we've done widget A, what is the commercial benefit of refactoring module B, which customers we expect to bring with feature C, and which business we retained by resolving defect D.


A speech may go like this:


<Slide 1: Screenshot of customisable skin engine, i.e. widget A>
We have been getting more and more customers requests for co-branding, and other modifications of our visual interface. We have prospects X and Y that depend on this capability, and there is anecdotal evidence, especially in the APAC market, of this being critical to remain competitive.
We have done the first steps there, and here how they look. Over the next few sprints we are going to expand in this direction.
<Slide 2: Mock-ups of future customisation>

<Slide 3: Statistics on recent logging issues, and list of escalations>

We had to invest some effort in refreshing our logging implementation. Unfortunately what we had created in 2012 has not scaled with the added demands in the past 3 years, and we had to deal with Z critical escalations diverting people from new features. For this reason, we decided to pay the refactoring price upfront: by spending 5 person-weeks, we expect to avoid customer calls in the future and reduce maintenance effort.

<Slide 4: High-level deployment schedule for telemetry, i.e. feature C>

For the past 3 months, we have been working across several teams on the new telemetry capabilities; this would help us in getting a better understanding of what users are doing and shaping future roadmap. This was a collaborative efforts between back-end, UI and middleware teams, and now we are getting it out to production. We had a small hiccup during deployment, but we're confident that it will be out by the end of this month. If you're interested in what specific telemetry we've created and how we'll use it for roadmap, I'm happy to go over a couple of extra slides.

How was this tailored to the audience?

  1. Strong emphasis on the commercial side, next steps and collaboration.
  2. Serial delivery without jumping across topics (as opposed to technical presentations to peers). With this type of presentation it's better to be predictable.
  3. Meaningful numbers. However, note that they should be the type of figures people will care about. The guys in the audience will recognise a meaningless stat when they see it.

  1. No mentioning of technology buzzwords (e.g. Docker). In some circumstances I might slip them in, but this will highly depend on who's present, and what I know of their current interests.
  2. Triggering dialogue and seeking feedback (see the last sentence in slide 3). This both shows that you are not over-confident, and gives proactive options on what your listeners would like to zoom in.

Summary

All in all, honesty, business understanding, good, well-prepared answers grounded on a "short-but-to-the-point" approach usually make the day.
Try and avoid slides for slides' sake, and do trigger opinions at key junctions. This type of presentation is an opportunity and the better you understand the agenda and what/why you are talking about, the less chances are there to go wrong.

Sunday 10 May 2015

Scrum - Part XII: Tracking sprints

Over the previous eleven installments, I talked about planning, periodic meetings, project management while straying here and there from the narrow path of classic Scrum. 

However, in this chapter I'm coming back all the way to where we started: running Agile sprints. To be more specific: figuring out the status of iterations and adjusting their course. 

Sorry, figuring out what?

Just to make sure you're still with me here, let's assume we're halfway into a 3-weekly sprint, and completed a third of the committed user stories. Our goal is getting a good projection of how many will be done when the clock strikes 5 PM next Friday. 

In case your "What" question has been replaced by "Why":

  • Other teams and customers depend on our commitments. If we break those, we at least should at least avoid delivering bad news at the eleventh hour.
  • Our own sprint plans depend on what we achieved. If we have to push tasks back at the last moment, then future sprints will exemplify a nice domino effect.

Well, having answered the What and Why (which to be fair, may not have been asked), it's easy to pull out the answer to How. Why don't you use the...

Burndown chart

Here's the thing: more often than not when seeing burndown charts, I heard one of the two variants of this phrase: "It is better/worse than it looks".

Of course, the experience might have been different for you, but let me expand a little bit first. There are two ways of burning down sprints: (1) by logging time, (2) by completing tasks.
At the risk of stating the obvious: with the former, the burndown chart goes whenever someone logs time, and with the latter whenever a task gets done.

Logging time - worse than it looks

This is the most natural way of tracking progress; we have estimated a task at 3 days - and we log a day each on Monday, Tuesday and Wednesday.

In this idealised world, the burndown chart looks like this:




Unfortunately, the more sprints we put behind us, the less idealistic the world looks. 

The crucial point is that I care far more about time remaining rather than time spent. Perhaps someone, somewhere, somehow, repeatedly estimates tasks right and completes them at an even pace, but in reality, it is not unheard of to estimate a task at 3 days and take a week and a half to complete. This is just how software development is - as hard as we try, we can't always avoid surprises.

So, by the time Wednesday turns into Thursday, the metrics look great - 3 days logged. In fact, the situation is dire: we are going to spend another week on the task, and most likely push a ton of stuff into the next sprint.
However, why don't we ask engineers to update time remaining? Surely, they know how much work is still outstanding, and can adjust the estimate accordingly.


Well, they don't always know. If you spent enough time in the industry, you must have heard the dreaded phrase "90% complete" only to discover that the remaining 10% have a life of their own.

This is the reason time logging paints too good a picture. It usually shows how much time is spent on tasks, which does not necessarily help with figuring out the health of a sprint.

Logging tasks - better than it looks


A more traditional school of thought is burning the sprint down only when tasks get completed.

This has a strong statement behind it: I do not care whether 80% or 67.5% of a task is complete; I only care when its acceptance criteria are ticked of, and it is declared as done.

It's definitely more honest, but it does have a couple of weaknesses:

a) It does not cope well with large stories. There's no insight from the burndown chart on how they're getting on, and the longer the story is, the more opaque it becomes. In worst cases, we end up getting the sordid picture below:




b) It is susceptible to minor roadblocks. E.g. a defect is solved, committed, and ready to be validated, but there is no environment to validate it on; in the meantime, the burndown chart flatlines.

However, honesty trumps visibility, while, on the other hand, these weaknesses can be overcome.

For starters, there are many other good reasons to avoid large tasks. They usually are a symptom of planning gaps, so one could argue that with proper homework, we don't need to worry about tracking those, and problem a does not exist.

As for minor roadblocks: there is a variety of tactics for getting them out from the way, but no single recipe. I usually prefer constructing stories with as few external dependencies as possible exactly for tracking purposes.

Another benefit of burning down only on task completion - it's less onerous on the engineers. This way they spend less time on bookkeeping and more time on doing useful work.

Thus, with everything taken into account, the only benefit from logging time is retrospectives; i.e. figuring out where we deviated from the original estimate. From my experience, it is hardly worth all the trouble, especially as overspend usually already manifests itself indirectly via tasks delayed to the next sprint. 

Stories versus sub-tasks

Just a bit of mechanics before wrapping up. All Scrum tools provide several layers of tasks:
  1. Epics (which I talked about in the past)
  2. Stories
  3. Subtasks

Subtasks differ from stories in three ways:
  • They are created and owned by the engineer, as opposed to the Product Owner 
  • Acceptance criteria on them are optional
  • They must be a part of a bigger story.
Just to make this a bit more visual, here's a sample JIRA screenshot:

For example, if our story is "Create a REST-ful service for data provisioning", its sub-tasks might be:
  • Define a mock API
  • Create auto-tests to drive the mock API (let's assume we are doing TDD)
  • Implement call A
  • Implement call B

Subtasks look perfectly suited for getting fine-grained units in place to drive burndown charts and proper planning. They are owned by the developer (so, Product Owner does not have to deal with umpteen user stories), and they are fine-grained by definition.

Unfortunately, I'm yet to find a tool which burns down on subtask completion. The usual reasoning is that nobody apart from the engineer should care about internal tasks being done.
My personal take is that others do care, and having those reflected in charts makes sprints less opaque, but we have to work with what we've got.

In short, subtasks look great in theory, but in practice they are impossible to track, and with time I ended up reverting to stories throughout.

To tame a chart

Getting the burndown graph to reflect reality is equally hard and important. The main recipe is having small tasks, and ticking things off only when they are done, not when they are complete. Subtask is the unfortunate victim of this approach, as Scrum tools do not reflect them in reports.

Sunday 3 May 2015

Reviewing Code: Theory in practice

As promised last time, I'll apply some of the review practices on a victim code sample.

The task at hand is generating a few pseudo-random passwords, and the code we're going to review is below (yep, it's C++ - home away from home):

#include <iostream>
#include <cstdlib>
#include <ctime>

const int MAX = 90;
const int MIN = 65;

char * createPassword();

int main()
{
    char * p = createPassword();    
    return 0;
}

char * createPassword()
{
    unsigned seed = time(0);

    srand(seed);

    char x = ' ';
    int passwordLength = 0;
    int numOfPasswords = 0;

    std::cout << "How many chars in password?";
    std::cin >> passwordLength;
    char * pwptr = new char[passwordLength];

    std::cout << "How many passwords should be generated?";
    std::cin >> numOfPasswords;

    int passwordcount = 0;

    do{
        for(int cnt = 0; cnt < passwordLength; cnt++)
        {
            x = (rand() % (MAX - MIN + 1)) + MIN;
            pwptr[cnt] = x;
            std::cout << pwptr[cnt];
        }
        std::cout << std::endl;
        passwordcount++;
        } while(passwordcount != numOfPasswords);

        return pwptr;
}
I've specifically selected a reasonable and functional fragment, so that we remain attached to reality. Let's recall the review order, striking out the stages that do not apply here:


  • Code reuse (or lack thereof)
  • Encapsulation, interfaces and self explanatory variable names. 
  • Correctness, i.e. no obvious defects with inverted conditions, missed basic use cases etc.
  • Performance and efficiency
  • More esoteric errors, such as off-by-ones, imprecise variable types, signed/unsigned, proper variable scope and so on
  • Auto-tests and code coverage
  • Correct merges to release branches and external dependencies.

Well, auto-tests might well apply, as they are conspicuous by their absence, but I'll play the usual card of brevity in the name of clarity.
Otherwise, there's not much room for reuse (we are not a part of a bigger project), or performance (pseudo-random number generation takes the lion share of the processing).

Now, if you have C++ baggage and a bit of time to spare, try and do your own review; we'll compare notes as we go along. 


Encapsulation and interfaces

Immediate observation: the function is called createPassword , but in fact it performs queryUserAndCreatePasswords.
It makes a lot of sense to separate creation of a single password, and remove user interaction. After all, if someone uses this function externally, they would not expect it to strike up a conversation with the user. 

This needs to be fixed before we move on, as lack of encapsulation might hide other issues. Rotating the table, and becoming the coder for 5 minutes, I end up with iteration number two:

#include <iostream>
#include <cstdlib>
#include <ctime>

void generatePasswords(int passwordLength, int numOfPasswords);
void generatePassword(int passwordLength, char *pwptr);

int main()
    {
    int passwordLength = 0;
    int numOfPasswords = 0;
    std::cout << "How many chars in password?";
    std::cin >> passwordLength;
 
    std::cout << "How many passwords should be generated?";
    std::cin >> numOfPasswords; 
 
    generatePasswords(passwordLength, numOfPasswords);
    return 0;
    }

void generatePasswords(int passwordLength, int numOfPasswords)
    {
    unsigned seed = time(0);
    srand(seed);
 
    char * pwptr = new char[passwordLength];
    for (int i = 0; i < numOfPasswords; ++i)
       {
       generatePassword(passwordLength, pwptr);
       std::cout << pwptr << std::endl;
       }
    }

void generatePassword(int passwordLength, char *pwptr)
    {
    const int MAX = 90;
    const int MIN = 65;
 
    char x = ' ';
    for(int cnt = 0; cnt < passwordLength; cnt++)
        {
        x = (rand() % (MAX - MIN + 1)) + MIN;
        pwptr[cnt] = x;
        }
    }
User interaction is moved to main(), and we now have a passable interface. People may rightfully frown upon such similar names as generatePassword and generatePasswords. My justification is that only generatePassword (in singular) is a candidate for an external interface, so we do not need to dwell too much on the other function's name.

Now, that we have a proper interfaces in place, let's look for 

Correctness

Jumping right into the crux of that matter: it might not look obvious, but the random passwords are not that random! 

Rand() creates predictable sequences if seeded with the same timestamp. So, if we invoke generatePasswords multiple times within the same second, we'll get the same sequence, and thus, the same set of passwords. Not good.

You might point out that with user interaction in place, there's not much chance of multiple password sets being requested within the same second. 
True, however we hope to reuse password generation in other, more automated, contexts by providing it as an interface. Naturally, we can't expect the callers to avoid calling the function twice in the same second.

This provides a nice opportunity for using random device facilities in C+11. Not only are they using hardware for generating entropy, they also have a convenient interface for specifying ranges and thus, casting the modulo function into oblivion. Without further ado, here's the third iteration: 
#include <iostream>
#include <random>

void generatePasswords(int passwordLength, int numOfPasswords);
void generatePassword(int passwordLength, char *pwptr);

int main()
    {
    int passwordLength = 0;
    int numOfPasswords = 0;
    std::cout << "How many chars in password?";
    std::cin >> passwordLength;
 
    std::cout << "How many passwords should be generated?";
    std::cin >> numOfPasswords; 
 
    generatePasswords(passwordLength, numOfPasswords);
    return 0;
    }

void generatePasswords(int passwordLength, int numOfPasswords)
    {
    char * pwptr = new char[passwordLength];
    for (int i = 0; i < numOfPasswords; ++i)
        {
        generatePassword(passwordLength, pwptr);
        std::cout << pwptr << std::endl;
        }
    }

void generatePassword(int passwordLength, char *pwptr)
    {
    const int MAX = 90;
    const int MIN = 65;
    std::random_device randomDevice;
    std::uniform_int_distribution<int> distribution (MIN, MAX);
 
    for(int cnt = 0; cnt < passwordLength; cnt++)
        pwptr[cnt] = distribution(randomDevice);
    }

General code quality

Now that we have both encapsulation and correctness sorted, let's look for more intricate defects.

The memory leak in line 23 is a case in point, and should be eliminated (just to state the obvious, pwptr is not freed).
Moreover, it's not just a memory leak - each subsequent password generation wipes out the previous token.

Additionally, we cannot have negative password lengths or counts, and the passwords themselves contain chars.

With all of that, we get iteration no. 4:
#include <iostream>
#include <iterator>
#include <random>
#include <vector>
#include <string>
#include <algorithm>

using PasswordList = std::vector<std::string>;
std::string generatePassword(size_t passwordLength);

int main()
    {
    size_t passwordLength = 0;
    size_t numOfPasswords = 0;
    std::cout << "How many chars in password?";
    std::cin >> passwordLength;
 
    std::cout << "How many passwords should be generated?";
    std::cin >> numOfPasswords; 
 
    PasswordList passwordList;
    std::generate_n( std::back_inserter(passwordList), numOfPasswords,
      [passwordLength] () { return generatePassword(passwordLength); });

    std::copy(passwordList.begin(), passwordList.end(),
              std::ostream_iterator<std::string>(std::cout, "\n"));
 
    return 0;
    }

std::string generatePassword(size_t passwordLength)
    {
    static const char MAX = 'Z', MIN = 'A';
    std::random_device randomDevice;
    std::uniform_int_distribution<int> distribution (MIN, MAX);
 
    std::string res(passwordLength, 0);
 
    for(size_t i = 0; i < passwordLength; ++i)
        res[i] = distribution(randomDevice);
 
    return res;
    }

There are several code improvements:

  • As promised, generatePasswords is gone due to C++ conveniently providing generate_n.
  • generatePassword now returns an std::string, hence we need not worry about memory leaks, allocations etc. If you're concerned about unnecessary string copes, have a look at RVO.
  • Generated passwords are printed using std::copy to an ostream_iterator.
  • Various declarations and definitions are tidied up: int became size_t; rather than having a magic number 65, I'm using 'A' and so on.

Hitting the Approval button


At this point, I finally pass the review. This was a fairly easy task, and the starting point wasn't even half bad. 
Nevertheless, my chances of hitting all the issues including memory leaks, wrong variable types, time sensitivity in a single pass would have been low.
So, having separate incremental review and rewriting stages helped a lot in flushing the issues out one by one.