In this multipart series, I take some perfectly working code which is definitely not clean and refactor it into a much cleaner and more readable implementation. If you disagree with my approach at any point then I ask that you voice your concerns in the comment section. In part 1 of this series, I introduced what I see as the problem and the code that I will be refactoring in subsequent posts. In part 2 of the series, I made the code a little more readable by refactoring it a little. The code was still not object-oriented, however, so here is part 3, in which we split the functionality up into a few different classes.
One major issue with the existing code is that the target solution is passed along at every stage. This means that on every turn of the game, the application must pass in the target as well as the guess. It would be much better if we could just pass that value once. We'll do that by creating a HangmanSolution class. This class will keep track of the target value as well as the partial solution.
[pre class="brush:java" title="HangmanSolution.java"]package com.bitsnibblesandbytes.hangman;
class HangmanSolution {
private String target;
private String partialSolution;
HangmanSolution(String target) {
target = this.target;
createPartialSolution();
}
String getTarget() {
return target;
}
String getPartialSolution() {
return partialSolution;
}
int checkCharacter(char guess) {
int count = 0;
for(int i = 0; i < target.length(); i++) {
if (updatePartialSolution(guess)) {
count++;
}
}
return count;
}
boolean isSolutionFound() {
return target.equalsIgnoreCase(partialSolution);
}
private boolean updatePartialSolution(char guess) {
boolean guessMatches = target.toLowerCase().charAt(i) == Character.toLowerCase(guess);
if (guessMatches) {
partialSolution = partialSolution.substring(0, i) + Character.toLowerCase(guess) + partialSolution.substring(i + 1);
}
return guessMatches;
}
private void createPartialSolution() {
StringBuilder builder = new StringBuilder();
for (int i = 0; i < target.length(); i++) {
builder.append('*');
}
partialSolution = builder.toString();
}
}
[/pre]
Most of the above logic is simply copied from the GamePlayer class we created in part 2. The one exception is the isSolutionFound() method. We will see where this method gets used a little later in this post. Note that I did not make this class or any of its methods public. This is because this class is simply meant to be used within this package, so package-private works. There's no point in adding something to my public API that I don't intend the public to use.
Another problem we have is that the GamePlayer class does everything. It should really just direct traffic as other classes do all of the heavy lifting. The HangmanSolution class starts us down the road to removing functionality. Let's move it a little further by pulling the guess validation code out into its own class:
[pre class="brush:java" title="GuessValidator.java"]package com.bitsnibblesandbytes.hangman;
class GuessValidator {
private String previousGuesses = "";
boolean isLetter(char guess) {
return Character.isAlphabetic(guess);
}
boolean isAvailableLetter(char guess) {
if (guesses.indexOf(Character.toLowerCase(guess)) < 0) {
guesses += Character.toLowerCase(guess);
return true;
}
return false;
}
}
[/pre]
Next, we'll pull out the logic for building the response string:
[pre class="brush:java" title="ResponseBuilder.java"]package com.bitsnibblesandbytes.hangman;
class ResponseBuilder {
static buildResponse(int count, char guess, HangmanSolution solution, boolean gameLost) {
String feedbackMessage = buildFeedbackMessage(guess, count);
String winLoseOrContinueMessage = buildWinLoseOrContinueMessage(solution, gameLost);
return feedbackMessage + "\n" + winLoseOrContinueMessage
}
private static String buildWinLoseOrContinueMessage(HangmanSolution solution, boolean gameLost) {
String winLoseOrContinue = "";
if (gameLost) {
winLoseOrContinue += "The word is " + solution.getTarget() + "\nYou lose.";
} else if (solution.isSolutionFound()) {
winLoseOrContinue += "The word is " + solution.getPartialSolution() + "\nYou win!";
} else {
winLoseOrContinue += solution.getPartialSolution();
}
return winLoseOrContinue;
}
private static String buildFeedbackMessage(char guess, int count) {
String verb;
String pluralModifier;
String number;
if (count == 0) {
number = "no";
verb = "are";
pluralModifier = "s";
} else if (count == 1) {
number = "1";
verb = "is";
pluralModifier = "";
} else {
number = Integer.toString(count);
verb = "are";
pluralModifier = "s";
}
return String.format("There %s %s %s%s", verb, number, guess, pluralModifier);
}
}
[/pre]
Note that the methods here are static since the ResponseBuilder does not depend on its own state at all. Some would find this a code smell, and I agree that this isn't the prettiest class, but at least it gets all of that string building out of the GamePlayer class, paring it down to a pretty short class:
[pre class="brush:java" title="GamePlayer.java"]package com.bitsnibblesandbytes.hangman;
public class GamePlayer {
private static final int MAX_ERRORS = 5;
private int errors = 0;
private HangmanSolution solution;
private GuessValidator validator;
public GamePlayer(String solution) {
this.solution = new HangmanSolution(solution);
validator = new GuessValidator();
}
public String handleGuess(char guess) {
if (!validator.isLetter(guess)) {
return "Error: you can only guess letters.";
}
if (!validator.isAvailableLetter(guess)) {
return "You already guessed that";
}
int count = solution.checkCharacter(guess);
if (count == 0) {
errors++;
}
return ResponseBuilder.buildResponse(count, guess, solution, errors > MAX_ERRORS);
}
}
[/pre]
This concludes the refactoring I'm going to do. It's not the most elegant solution, but without starting over from scratch we have at least achieved cleaner code than we started with.
In part 4 of the series, we'll add a simple feature quickly and easily that would not have been so easy to implement with the code we started with in part 1.
Wednesday, October 11, 2017
Thursday, July 13, 2017
Clean Code, part 2
In this multipart series, I take some perfectly working code which is definitely not clean and refactor it into a much cleaner and more readable implementation. If you disagree with my approach at any point then I ask that you voice your concerns in the comment section. In part 1 of this series, I introduced what I see as the problem and the code that I will be refactoring in subsequent posts. In this post, part 2 of the series, we make the code a little more readable by refactoring it a little. A lot of work will still be left to be done, however.
After listing the code in part 1 (which I will not list again here in the interest of space), I asked several questions. Let's start by looking at the answers to these questions:
I'm skipping the final question about enhancing the functionality of the game since the basic answer to it is "it would be very difficult". The less basic answer would be quite lengthy and out of the scope of this post. We'll return to it later in the series.
How long did it take you to figure out the answers to these questions? My guess is that it took a while. At the very least, you had to go through most of the code to answer the first two questions (and possibly the last one as well). Well written code should not require you to read through most or all of it in order to figure out what it does at a high level. Breaking the code up into functions will help, so we will begin there.
All of the functionality of this code is contained in a single function. How many different things does this function do? Well, it starts out by building the partial solution if it is currently null. (This should happen only the first time through, but it counts as something the function does.) The second thing it does is validate the input. Third, it checks to see if the guess has already been tried and if not then it adds the guess to the list of attempted guesses. Fourth, it checks whether the guess matches any letters in the solution or not. Fifth, it updates the partial solution. Sixth, it figures out the return message stating how many letters were matched by the guess. Finally, it adds a message to the return value explaining whether the player gets to make another guess and, if not, whether the player won or lost.
That one function does SEVEN things (and this is a conservative count). This seems a rather extreme violation of the Single Responsibility Principle. The least we could do is break it into functions that perform each of these steps. Our new code block may look something like this:
[pre class="brush:java" title="GamePlayer.java"]package com.bitsnibblesandbytes.hangman;
public class GamePlayer {
private static final int MAX_ERRORS = 5;
private int errors = 0;
private String guesses = "";
private String partialSolution;
public String takeTurn(char guess, String target) {
createPartialSolution(target);
if (isInvalidInput(guess)) {
return "Error: you can only guess letters.";
}
if (isAlreadyGuessed(guess)) {
return "You already guessed that";
}
int count = 0;
if (isCorrectGuess(target, guess)) {
count = updatePartialSolution(target, guess);
} else {
errors++;
}
return buildOutputMessage(target, guess, count);
}
private String buildOutputMessage(String target, char guess, int count) {
String feedbackMessage = buildFeedbackMessage(guess, count);
String winLoseOrContinueMessage = buildWinLoseOrContinueMessage(target);
return feedbackMessage + "\n" + winLoseOrContinueMessage
}
private String buildWinLoseOrContinueMessage(String target) {
String winLoseOrContinue = "";
if (errors > MAX_ERRORS) {
winLoseOrContinue += "The word is " + target + "\nYou lose.";
} else if (partialSolution.indexOf('*') < 0) {
winLoseOrContinue += "The word is " + partialSolution + "\nYou win!";
} else {
winLoseOrContinue += partialSolution;
}
return winLoseOrContinue;
}
private String buildFeedbackMessage(char guess, int count) {
String verb;
String pluralModifier;
String number;
if (count == 0) {
number = "no";
verb = "are";
pluralModifier = "s";
} else if (count == 1) {
number = "1";
verb = "is";
pluralModifier = "";
} else {
number = Integer.toString(count);
verb = "are";
pluralModifier = "s";
}
return String.format("There %s %s %s%s", verb, number, guess, pluralModifier);
}
private int updatePartialSolution(String target, char guess) {
int count = 0;
for(int i = 0; i < target.length(); i++) {
if (target.toLowerCase().charAt(i) == Character.toLowerCase(guess)) {
count++;
partialSolution = partialSolution.substring(0, i) + Character.toLowerCase(guess) + partialSolution.substring(i + 1);
}
}
return count;
}
private boolean isCorrectGuess(String target, char guess) {
return target.toLowerCase().indexOf(Character.toLowerCase(guess)) > -1;
}
private boolean isAlreadyGuessed(char guess) {
if (guesses.indexOf(Character.toLowerCase(guess)) >= 0) {
return true;
}
guesses += Character.toLowerCase(guess);
return false;
}
private boolean isInvalidInput(char guess) {
return !Character.isAlphabetic(guess);
}
private void createPartialSolution(String target) {
if (partialSolution == null) {
StringBuilder builder = new StringBuilder();
for (int i = 0; i < target.length(); i++) {
builder.append('*');
}
partialSolution = builder.toString();
}
}
}
[/pre]
(Side note: The above code adds a package statement and removes the unused import that was in the previous code in addition to the discussed changes.)
The above code is still far from perfect. For one thing, it is still not particularly object-oriented (since the logic has not been altered at all, just pulled out into private methods), but at least the original method is more readable. The names of the new methods explain what they're doing, which helps with the high-level interpretation of the code. The mystery of the maximum number of errors is also dispelled through the use of the MAX_ERRORS constant, eliminating the use of a magic number in buildWinLoseOrContinueMessage(). The mystery of lines 36-52 in the original code is solved by the name of the buildFeedbackMessage() method. The purpose of partialSolution is still not particularly clear, though the updatePartialSolution() method implementation and the placement of the call to it in the code provide strong clues.
We have begun the process of making this code more readable and more maintainable, but we still have a ways to go. In part 3 of this series, we will explore a more object-oriented approach to the problem and again refactor this code to follow object-oriented principles more closely.
After listing the code in part 1 (which I will not list again here in the interest of space), I asked several questions. Let's start by looking at the answers to these questions:
- At a high level, what does this code do?
- It simulates taking a turn in a hangman-like game.
- How many errors can be made before losing?
- 5
- What are lines 36-52 doing?
- Composing a response message to be sent back to the game driver.
- What is the point of the instance variable partialSolution?
- It keeps track of the letters that have been guessed (and not guessed) so far.
I'm skipping the final question about enhancing the functionality of the game since the basic answer to it is "it would be very difficult". The less basic answer would be quite lengthy and out of the scope of this post. We'll return to it later in the series.
How long did it take you to figure out the answers to these questions? My guess is that it took a while. At the very least, you had to go through most of the code to answer the first two questions (and possibly the last one as well). Well written code should not require you to read through most or all of it in order to figure out what it does at a high level. Breaking the code up into functions will help, so we will begin there.
All of the functionality of this code is contained in a single function. How many different things does this function do? Well, it starts out by building the partial solution if it is currently null. (This should happen only the first time through, but it counts as something the function does.) The second thing it does is validate the input. Third, it checks to see if the guess has already been tried and if not then it adds the guess to the list of attempted guesses. Fourth, it checks whether the guess matches any letters in the solution or not. Fifth, it updates the partial solution. Sixth, it figures out the return message stating how many letters were matched by the guess. Finally, it adds a message to the return value explaining whether the player gets to make another guess and, if not, whether the player won or lost.
That one function does SEVEN things (and this is a conservative count). This seems a rather extreme violation of the Single Responsibility Principle. The least we could do is break it into functions that perform each of these steps. Our new code block may look something like this:
[pre class="brush:java" title="GamePlayer.java"]package com.bitsnibblesandbytes.hangman;
public class GamePlayer {
private static final int MAX_ERRORS = 5;
private int errors = 0;
private String guesses = "";
private String partialSolution;
public String takeTurn(char guess, String target) {
createPartialSolution(target);
if (isInvalidInput(guess)) {
return "Error: you can only guess letters.";
}
if (isAlreadyGuessed(guess)) {
return "You already guessed that";
}
int count = 0;
if (isCorrectGuess(target, guess)) {
count = updatePartialSolution(target, guess);
} else {
errors++;
}
return buildOutputMessage(target, guess, count);
}
private String buildOutputMessage(String target, char guess, int count) {
String feedbackMessage = buildFeedbackMessage(guess, count);
String winLoseOrContinueMessage = buildWinLoseOrContinueMessage(target);
return feedbackMessage + "\n" + winLoseOrContinueMessage
}
private String buildWinLoseOrContinueMessage(String target) {
String winLoseOrContinue = "";
if (errors > MAX_ERRORS) {
winLoseOrContinue += "The word is " + target + "\nYou lose.";
} else if (partialSolution.indexOf('*') < 0) {
winLoseOrContinue += "The word is " + partialSolution + "\nYou win!";
} else {
winLoseOrContinue += partialSolution;
}
return winLoseOrContinue;
}
private String buildFeedbackMessage(char guess, int count) {
String verb;
String pluralModifier;
String number;
if (count == 0) {
number = "no";
verb = "are";
pluralModifier = "s";
} else if (count == 1) {
number = "1";
verb = "is";
pluralModifier = "";
} else {
number = Integer.toString(count);
verb = "are";
pluralModifier = "s";
}
return String.format("There %s %s %s%s", verb, number, guess, pluralModifier);
}
private int updatePartialSolution(String target, char guess) {
int count = 0;
for(int i = 0; i < target.length(); i++) {
if (target.toLowerCase().charAt(i) == Character.toLowerCase(guess)) {
count++;
partialSolution = partialSolution.substring(0, i) + Character.toLowerCase(guess) + partialSolution.substring(i + 1);
}
}
return count;
}
private boolean isCorrectGuess(String target, char guess) {
return target.toLowerCase().indexOf(Character.toLowerCase(guess)) > -1;
}
private boolean isAlreadyGuessed(char guess) {
if (guesses.indexOf(Character.toLowerCase(guess)) >= 0) {
return true;
}
guesses += Character.toLowerCase(guess);
return false;
}
private boolean isInvalidInput(char guess) {
return !Character.isAlphabetic(guess);
}
private void createPartialSolution(String target) {
if (partialSolution == null) {
StringBuilder builder = new StringBuilder();
for (int i = 0; i < target.length(); i++) {
builder.append('*');
}
partialSolution = builder.toString();
}
}
}
[/pre]
(Side note: The above code adds a package statement and removes the unused import that was in the previous code in addition to the discussed changes.)
The above code is still far from perfect. For one thing, it is still not particularly object-oriented (since the logic has not been altered at all, just pulled out into private methods), but at least the original method is more readable. The names of the new methods explain what they're doing, which helps with the high-level interpretation of the code. The mystery of the maximum number of errors is also dispelled through the use of the MAX_ERRORS constant, eliminating the use of a magic number in buildWinLoseOrContinueMessage(). The mystery of lines 36-52 in the original code is solved by the name of the buildFeedbackMessage() method. The purpose of partialSolution is still not particularly clear, though the updatePartialSolution() method implementation and the placement of the call to it in the code provide strong clues.
We have begun the process of making this code more readable and more maintainable, but we still have a ways to go. In part 3 of this series, we will explore a more object-oriented approach to the problem and again refactor this code to follow object-oriented principles more closely.
Friday, June 9, 2017
Clean Code, part 1
In this multipart series, I will take some perfectly working code which is definitely not clean and refactor it into a much cleaner and more readable implementation. If you disagree with my approach at any point then I ask that you voice your concerns in the comment section. In this post, part 1 of the series, I introduce what I see as the problem and the code that I will be refactoring in subsequent posts.
Many amateur and professional software developers take on the attitude "if it ain't broke, don't fix it". In other words, they will write their code until it works and then they are done with it. After all, isn't the point to have working software? Tight company deadlines encourage this approach as well, as the perception is that there isn't time to refactor and clean the code. It is my position that even with tight deadlines, you don't have time not to clean your code. In the long run, just getting the code working and then moving on is going to come back to bite you.
Think about it. You work long and hard to get a new feature working. Once it's working, you deliver it and move on to the next feature. A few weeks later, the initial feature needs to be enhanced. Will you be the one to work on it? Maybe. If not, then pity the poor soul who has to untangle what took you days to tangle up in the first place. If so, then just hope that you can remember what it was that you were doing, because it's unlikely to be clear from the code.
Let's take a look at a simple example.
[pre class="brush:java" title="GamePlayer.java"]import java.util.Scanner;
public class GamePlayer {
private int errors = 0;
private String guesses = "";
private String partialSolution;
public String takeTurn(char guess, String target) {
if (partialSolution == null) {
StringBuilder builder = new StringBuilder();
for (int i = 0; i < target.length(); i++) {
builder.append('*');
}
partialSolution = builder.toString();
}
if (!Character.isAlphabetic(guess)) {
return "Error: you can only guess letters.";
}
if (guesses.indexOf(Character.toLowerCase(guess)) >= 0) {
return "You already guessed that";
}
guesses += Character.toLowerCase(guess);
if (target.toLowerCase().indexOf(Character.toLowerCase(guess)) == -1) {
errors++;
}
int count = 0;
for(int i = 0; i < target.length(); i++) {
if (target.toLowerCase().charAt(i) == Character.toLowerCase(guess)) {
count++;
partialSolution = partialSolution.substring(0, i) + Character.toLowerCase(guess) + partialSolution.substring(i + 1);
}
}
String verb;
String pluralModifier;
String number;
if (count == 0) {
number = "no";
verb = "are";
pluralModifier = "s";
} else if (count == 1) {
number = "1";
verb = "is";
pluralModifier = "";
} else {
number = Integer.toString(count);
verb = "are";
pluralModifier = "s";
}
String output = String.format("There %s %s %s%s", verb, number, guess, pluralModifier);
if (errors > 5) {
output += "\nThe word is " + target + "\nYou lose.";
} else if (partialSolution.indexOf('*') < 0) {
output += "\nThe word is " + partialSolution + "\nYou win!";
} else {
output += "\n" + partialSolution;
}
return output;
}
}[/pre]
Many amateur and professional software developers take on the attitude "if it ain't broke, don't fix it". In other words, they will write their code until it works and then they are done with it. After all, isn't the point to have working software? Tight company deadlines encourage this approach as well, as the perception is that there isn't time to refactor and clean the code. It is my position that even with tight deadlines, you don't have time not to clean your code. In the long run, just getting the code working and then moving on is going to come back to bite you.
Think about it. You work long and hard to get a new feature working. Once it's working, you deliver it and move on to the next feature. A few weeks later, the initial feature needs to be enhanced. Will you be the one to work on it? Maybe. If not, then pity the poor soul who has to untangle what took you days to tangle up in the first place. If so, then just hope that you can remember what it was that you were doing, because it's unlikely to be clear from the code.
Let's take a look at a simple example.
[pre class="brush:java" title="GamePlayer.java"]import java.util.Scanner;
public class GamePlayer {
private int errors = 0;
private String guesses = "";
private String partialSolution;
public String takeTurn(char guess, String target) {
if (partialSolution == null) {
StringBuilder builder = new StringBuilder();
for (int i = 0; i < target.length(); i++) {
builder.append('*');
}
partialSolution = builder.toString();
}
if (!Character.isAlphabetic(guess)) {
return "Error: you can only guess letters.";
}
if (guesses.indexOf(Character.toLowerCase(guess)) >= 0) {
return "You already guessed that";
}
guesses += Character.toLowerCase(guess);
if (target.toLowerCase().indexOf(Character.toLowerCase(guess)) == -1) {
errors++;
}
int count = 0;
for(int i = 0; i < target.length(); i++) {
if (target.toLowerCase().charAt(i) == Character.toLowerCase(guess)) {
count++;
partialSolution = partialSolution.substring(0, i) + Character.toLowerCase(guess) + partialSolution.substring(i + 1);
}
}
String verb;
String pluralModifier;
String number;
if (count == 0) {
number = "no";
verb = "are";
pluralModifier = "s";
} else if (count == 1) {
number = "1";
verb = "is";
pluralModifier = "";
} else {
number = Integer.toString(count);
verb = "are";
pluralModifier = "s";
}
String output = String.format("There %s %s %s%s", verb, number, guess, pluralModifier);
if (errors > 5) {
output += "\nThe word is " + target + "\nYou lose.";
} else if (partialSolution.indexOf('*') < 0) {
output += "\nThe word is " + partialSolution + "\nYou win!";
} else {
output += "\n" + partialSolution;
}
return output;
}
}[/pre]
If, at this point, you find yourself saying, "He said a simple example. This doesn't look simple," then I have made my point. This really is a simple example. It is functionally and syntactically correct. Please answer the following questions for yourself and pay attention to how long it takes to be able to do so.
- At a high level, what does this code do?
- How many errors can be made before losing?
- What are lines 36-52 doing?
- What is the point of the instance variable partialSolution?
- Suppose that the rules of the game are to be changed so that if the solution contains multiple copies of the same letter then they are filled in one at a time when that letter is guessed. For example, if the word was 'feelers' then the first guess of the letter 'e' would reveal the first 'e', but not the others (*e*****), the second guess of the letter 'e' would reveal the second 'e' (*ee****), and so on.
- Where would you start?
- How many points in the code would need to be touched?
- What if the new rule was to be optional, chosen by the player before starting the game?
The point I am making here is that the above questions are not easy to answer quickly. The code works, but it is not obvious what it does or what all its parts are for. This is for a fairly simple game written by one person in an hour or so. The problem gets exponentially worse for complex business domains written and maintained by multiple people on multiple teams over the course of years. This, my friends, is where spaghetti code comes from.
The code above is not only dirty, but it is also not object oriented. (Just because it's Java doesn't mean it's object oriented.) It is structured in that it doesn't have goto statements of any type (goto, continue, break), but it is very procedural. In part 2 of this series, I will start with this code and discuss how to make it cleaner and more object oriented.
The code above is not only dirty, but it is also not object oriented. (Just because it's Java doesn't mean it's object oriented.) It is structured in that it doesn't have goto statements of any type (goto, continue, break), but it is very procedural. In part 2 of this series, I will start with this code and discuss how to make it cleaner and more object oriented.
Wednesday, April 19, 2017
Options Besides Whiteboard Coding for Technical Interviews
In my last post, I gave some brief advice on how to pass a technical interview with whiteboard coding problems. I am not a fan of the whiteboard coding interview. Whiteboard coding is a very specific skill that really doesn't translate into the job well (or at all). I know plenty of highly skilled professionals who cannot pass a whiteboard coding interview and a fair number of not-so-highly-skilled "professionals" who are absolute wizards when it comes to whiteboard coding. So what can we do to determine if our candidates can read and write code that is more relevant to the job they are interviewing for?
I am a software engineer, so my ideas tend to be geared towards those who spend most of their time writing code. That said, here are some ideas for interviews that are more relevant than whiteboard coding. Please feel free to comment with additional thoughts and/or suggestions.
- Interpreting Code: Provide the candidate with a function that is written in their language of choice (or at least in a language they will be required to use should they get the position). This function could be on paper or presented on a computer screen (even more realistic). For the most realistic experience, the candidate should have access to resources that they may use in real life to learn what they don't already know. (Google, for example, or StackOverflow).
Now, well-written code that needs interpreting is not a frequent occurrence in a large code base (unfortunately), so this function should be written in a style that does not follow best practices, but does work. Unless the person is interviewing to work at the coder's Valhalla where all code is well written, this is much more realistic. The candidate must then determine what the code does.
- Refactoring Code: This could be done as a followup to the interpreting code question. Once the candidate has figured out what the code does then have them rewrite the code in a clean style, following the SOLID principles in the case of object-oriented programming, for example. I would recommend providing the candidate with an IDE (or at the very least a computer with a text editor) and just let them work on it for 10 or 15 minutes.
Don't have them do the refactoring on the whiteboard; that would defeat the purpose of doing this instead of whiteboard coding. If you can't provide the candidate with a computer to use then have them lay out what classes and functions they might create (again, speaking in OOP since that's what I'm most familiar with), but don't make them implement it without a computer.
- Reviewing Code: Provide the candidate with a class or function and have them review the code verbally. The code might be functional or it might not be. It might even be functional, but not meeting requirements that you provide the candidate with. You really have a lot of flexibility with how you could lay a problem like this out.
- Add a Feature: This is another exercise that could follow a code interpretation question or a code refactoring question. In fact, you might just have this question immediately follow the code interpretation and see whether the candidate refactors as they go or not.
If you haven't already, provide the candidate with existing code. It's up to the interviewer whether this is really clean code or not; I would suggest somewhere between really bad code and really clean code. Give the candidate the requirements for a simple feature to add to the existing code. Allow them to use an IDE to add that feature to the code.
In this case, I would provide them with a working project in an IDE--if I don't have they're favorite available then the least I can do is have the project all set up for them. Expect them to write tests if that's what you have your developers do. Have them do all the steps required in adding a feature to your product(s).
The biggest weakness I see in the above suggestions is the time involved in preparing. If you want the candidate to be able to work in the language of their choice then you'll have to have code translated into several different languages. This might be fun, though, as you could dip your hand into several different languages in the process. Since the code doesn't have to be pretty, you might even create a better exercise in a language you don't know well than in one you do! One useful resource for such a thing is https://rosettacode.org/wiki/Rosetta_Code. (This is a site that I could just sit and play with all day.)
Regardless of whether you believe whiteboard coding is a good measure of a candidate or not, these ideas provide a very few additional options that you can use to switch up your interview questions or to use in a longer interview where you have time for more than one technical question. Please feel free to share your thoughts or additional ideas in the comments.
Wednesday, April 5, 2017
Passing a Technical Interview
For many programmers, the technical interview is the hardest part to get past when searching for a new job. It's not that they don't know how to code, they do. It's also not that they don't know how to solve problems, they do. But the coding interview is a special animal that requires a special skill to pass. There is a large amount of debate about how useful a coding interview can be in determining the best candidate for a job. This post is not intended to go into that debate (though you're welcome to make any observations about the merits of coding interviews in the comments if you like). Here are some links you can take a look at if you want to see what people are saying:
- Why Technical Interviews Work (And Why They Don't)
- The Technical Interview Is Dead (And No One Should Mourn)
- Why we don't have technical interviews for technical roles at Buffer
- Programmers are confessing their coding sins to protest a broken job interview process
- The Rise And Looming Fall Of The Engineering Whiteboard Interview
- Is It Time to Kill the Whiteboard Interview?
You might notice that a fair number of these links are to articles about how the whiteboard interview is ineffective or failing. This isn't because of my bias; it's because there are a lot of articles out there questioning the efficacy of whiteboard coding interviews. Yet, in spite of the number of articles predicting the downfall of such interviews, they are still very much in the mainstream. It is very difficult to find a tech company that doesn't expect whiteboard coding (or, if over the phone, coding in a shared web environment where the interviewer can watch the interviewee's every keystroke) from each of its software developer candidates.
The lesson here is that if you are going to get a software development job then it is very likely that you'll need to pass at least one coding interview. Some companies will even do one or two coding interviews over the phone, then have you come on site and do three or four more on the whiteboard. This makes the ability to get through these interviews an important skill for landing a job, if not for actually doing the job. What follows are some tips (in no particular order) for how to get through a coding interview.
- Talk to the interviewer. The theory behind a coding interview is that the interviewer gets to see not only if you can write code or not, but also how you go about solving a problem. If you do all of your thinking inside your head then not only is the room (or phone line) filled with awkward silence, but the interviewer has no idea whatsoever what you're thinking or how you're working through the problem.
- Ask questions. Some interviewers will deliberately leave important information out of the question they ask you in order to determine whether you can identify the missing information and know to ask for it. Do not make assumptions about what they must have meant! This is a major tripping point for a lot of people, as the tendency is to dive into coding using assumptions that they don't even realize they've made. Instead, treat the interviewer as a collaborator and work with them in coming up with a solution.
- Write a few examples on the board before beginning to code. This not only helps clear up any misunderstandings that there may be about what the problem is asking for, but it also gives you something to test your code against as you go along.
- Practice a LOT before the interview. There are a lot of ways for you to get practice in, some free, some not. https://codefights.com/ allows you to test your skills against other people. https://www.meetup.com/ probably has meetups in your area in which people get together to practice coding interviews. There are also classes or coaching sessions, such as Technical Interview Mastery Workshop, put on by Blossom Career. (Full disclosure: Blossom Career is my wife's company and I help teach the workshop. There is a fee for this workshop that helps cover the cost of renting space for it.) The point here is to get a lot of practice, whether via one on one coaching, group coaching, practicing with others, or simply practicing on your own. (If you don't have a whiteboard, you can practice by writing your code on paper.) Make sure that you practice explaining what you're doing as well as solving the problems/writing the code.
One final note on this topic for you: there are lots of books out there that offer sample interview questions for you to practice with. By far the best book I have seen is Cracking the Coding Interview by Gayle Laakmann McDowell. This book not only contains sample questions and solutions, but it also contains chapters with excellent advice on getting through the interviewing process. I highly recommend this book. (I have no affiliation with the author or publisher of this book, I just think it is an excellent resource for you.)
Tuesday, March 28, 2017
Spring Configuration -- Do You Use a Component Scan?
If you use Spring beans for dependency injection in any of your applications then you know that you frequently have to define quite a lot of them. Whether you use xml configuration, Java annotation-based configuration, or a mix of the two, you still wind up creating a bean for each component of your application. With even a moderately complex system, this number of beans adds up very quickly. Thus, we have the component scan.
For those of you who aren't familiar with it, a component scan makes it very easy to create a new component. All you need to do is place an @Component tag on the class itself and then indicate in one of your configuration files that Spring should scan for components in the directory that your class is in.
A brief example:
First, define the component using the @Component tag.
[pre class="brush:java" title="MyComponent.java"]package my.component.pkg;
import org.springframework.stereotype.Component;
@Component
public class MyComponent {
...
}[/pre]
Next, tell Spring to scan for components. I'll give both the Java and xml configurations, but really only one is necessary.
[pre class="brush:java" title="MyConfiguration.java"]package my.configuration.pkg;
import my.component.pkg.MyComponent;
import org.springframework.context.annotation.*;
@Configuration
@ComponentScan("my.component.pkg")
public class MyConfiguration {
}[/pre]
[pre class="brush:xml" title="component-configuration.xml"]<beans xmlns="http://www.springframework.org/schema/beans"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xmlns:context="http://www.springframework.org/schema/context"
xsi:schemaLocation="http://www.springframework.org/schema/beans
http://www.springframework.org/schema/beans/spring-beans-4.3.7.xsd
http://www.springframework.org/schema/context
http://www.springframework.org/schema/context/spring-context-4.3.7.xsd">
<context:component-scan base-package="my.component.pkg" />
</beans>[/pre]
Pretty nifty, huh? Well, maybe. Until you try to run a bunch of integration tests. You see, I might have hundreds (or even thousands) of components defined this way (within the same package or in multiple base packages). Now suppose I want to run an integration test on MyComponent. When Spring starts up, it loads not just MyComponent, but every component in your application. This is not only unnecessary, but for a very large application, it can take up to a minute, possibly more, before your test can even begin running.
The above issue has led my team recently to abandoning using the component scan. We have begun defining beans using the @Bean tag in a Java configuration file in the same package as the component. The above example would then look like this:
[pre class="brush:java" title="MyComponent.java"]package my.component.pkg;
class MyComponent {
...
}[/pre]
[pre class="brush:java" title="ComponentConfig.java"]package my.component.pkg;
import org.springframework.context.annotation.*;
@Configuration
public class ComponentConfig {
@Bean
MyComponent myComponent() {
return new MyComponent();
}
..
}[/pre]
We would define all components in the my.component.pkg package in this configuration file. Then, when running a test on MyComponent, I only need to load this configuration and can skip loading the components that are not needed for the test.
This finally brings me to my question: do you/would you use component scan to simplify the creation of component beans or do you/would you avoid it in order to be able to more easily control how much of the application context must be loaded when running a test? Please leave a response in the comments.
For those of you who aren't familiar with it, a component scan makes it very easy to create a new component. All you need to do is place an @Component tag on the class itself and then indicate in one of your configuration files that Spring should scan for components in the directory that your class is in.
A brief example:
First, define the component using the @Component tag.
[pre class="brush:java" title="MyComponent.java"]package my.component.pkg;
import org.springframework.stereotype.Component;
@Component
public class MyComponent {
...
}[/pre]
Next, tell Spring to scan for components. I'll give both the Java and xml configurations, but really only one is necessary.
[pre class="brush:java" title="MyConfiguration.java"]package my.configuration.pkg;
import my.component.pkg.MyComponent;
import org.springframework.context.annotation.*;
@Configuration
@ComponentScan("my.component.pkg")
public class MyConfiguration {
}[/pre]
[pre class="brush:xml" title="component-configuration.xml"]<beans xmlns="http://www.springframework.org/schema/beans"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xmlns:context="http://www.springframework.org/schema/context"
xsi:schemaLocation="http://www.springframework.org/schema/beans
http://www.springframework.org/schema/beans/spring-beans-4.3.7.xsd
http://www.springframework.org/schema/context
http://www.springframework.org/schema/context/spring-context-4.3.7.xsd">
<context:component-scan base-package="my.component.pkg" />
</beans>[/pre]
Pretty nifty, huh? Well, maybe. Until you try to run a bunch of integration tests. You see, I might have hundreds (or even thousands) of components defined this way (within the same package or in multiple base packages). Now suppose I want to run an integration test on MyComponent. When Spring starts up, it loads not just MyComponent, but every component in your application. This is not only unnecessary, but for a very large application, it can take up to a minute, possibly more, before your test can even begin running.
The above issue has led my team recently to abandoning using the component scan. We have begun defining beans using the @Bean tag in a Java configuration file in the same package as the component. The above example would then look like this:
[pre class="brush:java" title="MyComponent.java"]package my.component.pkg;
class MyComponent {
...
}[/pre]
[pre class="brush:java" title="ComponentConfig.java"]package my.component.pkg;
import org.springframework.context.annotation.*;
@Configuration
public class ComponentConfig {
@Bean
MyComponent myComponent() {
return new MyComponent();
}
..
}[/pre]
We would define all components in the my.component.pkg package in this configuration file. Then, when running a test on MyComponent, I only need to load this configuration and can skip loading the components that are not needed for the test.
This finally brings me to my question: do you/would you use component scan to simplify the creation of component beans or do you/would you avoid it in order to be able to more easily control how much of the application context must be loaded when running a test? Please leave a response in the comments.
Friday, March 10, 2017
The final keyword in Java
Most experienced Java developers are familiar with the final keyword. It can be used at three different levels: class, method, and variable (or field).
Probably the most common use of final is in the definition of constants, such as
Probably the most common use of final is in the definition of constants, such as
public static final Integer MY_CONSTANT = 475;
Another common use is for local variables (or method parameters) whose value never changes:
String getNameAndAge() { final String name = determineName(); final Integer age = determineAge(); return name + " " + age; }
That the values of name and age in the above example cannot be changed leads some to believe that the final keyword makes an object immutable. In a code review I was doing recently, I came across a snippet of code similar to the following:
// make the context immutable final Context context = contextFactory.build(contextParameters);
The author had been told in a previous comment to make Context immutable, and this was their attempt to do so. To understand why this fails to make context immutable, it helps to recall how Java stores data.
Briefly, for primitive values, the value is stored directly in the stack. For the sake of visualization, we can say that the variable stores the value itself. For objects, however, the variable stores not the object, but a reference to the object. In other words, adding the final keyword only limits you from reassigning the reference that is stored by the variable.
This means that the final keyword in the above case means that you cannot assign a different Context instance to context, but you can use the setter methods in the Context class to modify the instance that you do have, so you do not have an immutable instance of Context. The way to make Context immutable is not in the use of final, but instead in removing the setter methods from the Context class itself. You cannot make an immutable instance of a class that is itself mutable.
As mentioned above, final can be used at the class level and at the method level as well. When used at the class level, it means that the class cannot be extended (see java.lang.String). When used at the method level, it means that the method cannot be overridden.
There are style issues around final as well, such as whether or not to label method parameters as final or whether final should be used on local variables at all. Style is a bit outside the scope of this article, but I urge you (and your team) to have a consistently used policy.
This means that the final keyword in the above case means that you cannot assign a different Context instance to context, but you can use the setter methods in the Context class to modify the instance that you do have, so you do not have an immutable instance of Context. The way to make Context immutable is not in the use of final, but instead in removing the setter methods from the Context class itself. You cannot make an immutable instance of a class that is itself mutable.
As mentioned above, final can be used at the class level and at the method level as well. When used at the class level, it means that the class cannot be extended (see java.lang.String). When used at the method level, it means that the method cannot be overridden.
There are style issues around final as well, such as whether or not to label method parameters as final or whether final should be used on local variables at all. Style is a bit outside the scope of this article, but I urge you (and your team) to have a consistently used policy.
Subscribe to:
Posts (Atom)