Are you new to code reviews? Do you know what to look for in a code review? Do you feel frustrated with your code review? I’ve been there too. Let’s see some tips I learned and found to improve our code reviews.
Code review is a stage of the software development process where a piece of code is examined to find bugs, security flaws, and other issues. Often reviewers follow a coding standard and style guide while reviewing code.
TL;DR
For the reviewer: Be nice. Remember you are reviewing the code, not the writer.
For the reviewee: Don’t take it personally. Every code review is an opportunity to learn.
For all the dev team: Reviews take time too. Add them to your estimates.
Advantages of Code Reviews
Code reviews are a great tool to identify bugs before the code gets shipped to end users. Sometimes we only need another pair of eyes to spot unnoticed issues in our code.
Also, code reviews ensure that the quality of the code doesn’t degrade as the project moves forward. They help to spread knowledge inside a team and mentor new members.
Now that we know what code reviews are good for, let’s see what to look for during code reviews and tips for each role in the review process.
What to look for in a code review?
If we’re new to code reviews and we don’t know what it’s going to be reviewed in our code…or if we have been asked to review somebody else code and we don’t know what to look for, we can start looking at this:
Does the code:
Compile in somebody else machine? If you have a Continuous Integration/Continuous Deployment (CI/CD) tool, we can easily check if the code is compiling and all tests are passing.
Include unit or integration tests?
Introduce new bugs?
Follow current standards?
Reimplement things? Is some logic already implemented in the standard library or an extension method?
Build things the hard way?
Kill performance?
Have duplication? Has the code been copied and pasted?
It’s a good idea to have a checklist next to us while reviewing the code. We can create our own checklist or use somebody else as a reference. Like Doctor McKayla Code Review Checklist.
Before we start any review, let’s understand the context around the code we’re about to review.
A good idea is to start by looking at the unit tests and look at the “diff” of the code twice. One for the general picture and another one for the details.
If we’re a code reviewer, let’s:
Be humble. We all have something to learn.
Take out the person when giving feedback. We are reviewing the code, not the author.
Be clear. We may review code from juniors, mid-level, or seniors, even from non-native speakers of our language. Everybody has different levels of experience. Obvious things for us aren’t obvious for somebody else.
Give actionable comments. Let’s not use tricky questions to make the author change something. Let’s give clear and actionable comments instead. For example, what do you think about this method name? vs I think X would be a better name for this method. Could we change it?
Always give at least one positive remark. For example: It looks good to me (LGTM), good choice of names.
Use questions instead of commands or orders. For example, Could this be changed? vs Change it.
Use “we” instead of “you”. We’re part of the development process too. We’re also responsible for the code we’re reviewing.
Instead of showing an ugly code, teach. Let’s link to resources to explain even more. For example, blog posts and StackOverflow questions.
Review only the code that has changed. Let’s stop saying things like Now you’re here, change that method over there too.
Find bugs instead of style issues. Let’s rely on linters, compiler warnings, and IDE extensions to find styling issues.
Recently, I found out about Conventional Comments. With this convention, we start our comments with labels to show the type of comments (suggestion, nitpick, question) and their nature (blocking, non-blocking, if-minor).
Before asking someone to review our code, let’s review our own code. For example, let’s check if we wrote enough tests and followed the naming conventions and styling guidelines.
It’s a good idea to wait for the CI/CD to build and run all tests before asking someone to review our changes or assign reviewers in a web tool. This will save time for our reviewers and us.
If we’re a reviewee, let’s:
Stop taking it personally. It’s the code under review, not us.
Give context. Let’s give enough context to our reviews. We can write an explanatory title and a description of what our code does and what decisions we made.
Keep your work short and focused. Let’s not make reviewers go through thousands of lines of code in a single review session. For example, we can separate changes in business logic from formatting/styling.
Keep all the discussion online. If we contact reviewers by chat or email, let’s bring relevant comments to the reviewing tool for others to see them.
For team management
If we’re on the management side, let’s:
Make code reviews have the highest priority. We don’t want to wait days until we get our code reviewed.
Remember code reviews are as important as writing code. They take time too. Let’s add them to our estimates.
Have as a reviewer someone familiar with the code being reviewed. Otherwise, we will get styling and formatting comments. People judge what they know. That’s a cognitive bias.
Have at least two reviewers. For example, as reviewees, let’s pick the first reviewer. Then he will choose another one until the two of them agree.
Voilà! These are the tips I’ve learned while reviewing other people’s code and getting mine reviewed too. Code reviews can be frustrating. Especially when they end up being a discussion about styling issues and naming variables. I know, I’ve been there.
One of my lessons as a code reviewer is to use short and focused review sessions. I prefer to have short sessions in a day than a single long session that drains all my energy. Also, I include a suggestion or example of the change to be made in every comment. I want to leave clear and actionable comments on every code review.
You have heard about Pythonic code? Languages have an expressive or “native” way of doing things. But, what about C#? Is there C-Sharpic code?
In this series of posts, I will attempt to present some idioms or “expressions” to write more expressive C# code. I collected these idioms after reviewing code and getting mine reviewed too.
In this first part, you have two useful C# idioms on conditionals and its alternative solutions.
Instead of lots of or’s, use an array of possible values
Use an array of known or valid options, instead of a bunch of comparison inside an if statement.
Use this idiom when checking preconditions or validating objects.
Before, we used comparison with || inside an if statement.
If you need to check for a new value, you add it in the array instead of adding a new condition in the if statement.
Instead of lots of if’s to find a value, use an array of Func
Replace consecutive if statements to find a value with an array of Func or small choice functions. Then, pick the first result different from a default value or null.
Use this idiom when finding a value among multiple choices.
Similarly, if you need to add a new alternative, either you add it in the array or nest it instead of adding the new alternative in the if statement.
Validate a complex object with an array of Func
Also, this last idiom is useful when validating an object against a list of rules or conditions. Create a function for every rule, then use All() LINQ method to find if the input object or value satisfy all required rules.
Voilà! These are our first two idioms on conditionals. I have found these two idioms more readable in some scenarios. But, don’t start to rewrite or refactor your code to follow any convention you find online. Make sure to follow the conventions in your own codebase, first.
Dear developer, you’re working in a project that uses Team Foundation Server, TFS. You’re used to check-out and check-in your files. Now you have to use Git. Do you know how the two relate to each other? This is what makes TFS and Git different.
Team Foundation Server (TFS) and Git belong to two different types of Version Control Systems. TFS is centralized and Git is distributed. This distinction makes collaborating with others, syncing changes and branching different.
Centralized vs Distributed
Centralized version control systems need a server in place to operate. To version control a file, view the history of a file or perform any other operation you need to communicate to a server.
But, distributed version control systems don’t need a server. Each operation is performed in a local copy of the project. You could choose to sync your local copy with a server later on.
TFS is a centralized version control system and Git a distributed one.
TFS vs Git
This distinction between Centralized and Distributed Version Control Systems brings some differences between TFS and Git. These are some of them.
Exclusive checkout
Have you ever needed to modify a file and you couldn’t because a co-worker was working on that file too? That’s Exclusive Checkout.
Exclusive Checkout is a feature you can turn on or off in TFS. But, it isn’t available in Git. Because, there is no lock on files to have only one user working in a file at a time.
With Git, each developer works in his own copy of the project.
No mappings
Before starting to work with a project under TFS, you need some “mappings”. A place where the files in TFS server will be in your computer.
With Git, you can clone and keep a project anywhere in your computer. There’s no such a thing as mappings.
Checking and Pushing
After modifying some files, you want to version control them. With TFS you “check-in” your files. This means, those files are kept in the history of the project and synced with the server.
With Git, you don’t “check-in” your files, you “commit” them. And, your committed files live only in your local copy of the project, until you sync or push them to a server.
Branching
With Git, branches have a new meaning. You could have lots of light-weight and short-lived branches to try things out, solve bugs or do your everyday work. By convention, all Git repositories start with a branch called master.
Starting from Git 2.28, Git will use the configuration value init.defaultBranch to name the default branch. Other alternatives for master are: main, primary or default.
Gitflow, a branching model
Maybe, you have worked with one branch per environment: Dev, QA, Beta and Production. You start every new task directly on the Dev branch.
There is another branch model: Gitflow.
Gitflow suggests that the master branch mirrors the Production environment. The develop branch is where everyday work happens. But, every new task starts in a new branch taken from develop. Once you’re done with your task, you merge this branch back to develop.
In case of bugs or issues in Production, you create a new branch from master. And you merge it back to master and develop once you’re done fixing the issue.
Every release has a separate branch, too. This release branch is also merged to master and develop. For more details, see Introducing GitFlow.
Pull or Merge Requests
Some projects adopt another convention. Nobody pushes or syncs directly to the master or develop branch.
Instead, every new task goes through a code review phase using a pull or merge request. During code review, your code is examined to find bugs and to stick to coding conventions.
A pull or merge request means that “the author of the task asks to merge his changes from a branch into the current codebase”.
Most of the time, this code review is done through a web tool or web interface. After one or two team members review your changes, the same tool will allow the reviewer to merge the changes to the destination branch.
If you're interested in the code review process, check my post on Tips and Tricks for Better Code Reviews. It contains tips for everyone involved in the code review process.
Reference
This is how TFS and Git actions relate to each other.
Changesets = Commits
In TFS, a changeset is a set of changes that have been version-controled and synced to the server.
With Git, you have commits. Your commits can live only on your local copy, until you upload them to a server.
If you want to collaborate with others or host your code somewhere else, you can use a server as a single point of authority.
Have you ever heard about GitHub, GitLab or Bitbucket? These are third-party hosting solutions for Git.
Check-in = Commit + Push
With TFS, you “check-in” your files. But, with Git, from the command line, you need three commands: add, commit and push.
To get the most recent changes from a server, with TFS, you “get latest version”.
With Git, you need to “pull” from a remote branch on a server. Use git pull origin my-branch.
By convention, the name associated to the sync server is origin. You can add other remotes if needed.
Branch = Branch + Checkout
With TFS, you create a new branch from the source branch using the “Branch” option. If you have a large project, this could take a while.
With Git, you need to create a branch and switch to it. Use the commands: branch and checkout, respectively. But, you can combine these two actions into a single one with checkout and the -b flag.
$ git checkout -b a-new-branch
Shelve = stash
If you want to temporary suspend your work and resume it later, you use a shelve in TFS or an stash with Git.
With Git, to create an stash, use git stash -u. And, git stash apply to bring back your changes.
Integration
Please, do not be afraid of using the command line and memorizing lots of commands. There are Git clients and out-of-the-box integrations (or plugins) for Git in most popular IDE’s. For example, Visual Studio and Visual Studio Code support Git out-of-the-box, and SourceTree is a Git client you can download for free.
Voilà! That’s what makes Git and TFS different. Now you know where your changesets are, how to check-out to files and why you don’t need any mappings with Git.
Are you applying for a remote position? For a on-site position but the interviewer is on another branch outside your city? Are preparing yourself for future interviews? Let’s see 4 types of interview and tips to prepare for them.
There are four types of interviews you can find while applying for a new job.
1. Conversation
In conversational interviews, the interviewer will start a casual conversation about your experience, your past positions and your skills. Be ready to describe one or two past projects.
When describing your projects, make sure to include:
Name of the project
Purpose and Market
Duration: How long did it take to finish it? or how long were you in it?
Tech Stack: what frameworks and tools did you use on the front-end and the back-end?
Team Size: Did you have a QA team? designers?
Role and areas in charge: What did you do inside the project? What were you responsible for?
Biggest challenge you faced in the project: a new tech stack? changing requirements? short duration?
2. Interrogation
In interrogational interviews, the interviewer will have a list of predefined questions to test your knowledge on the skills required for the position.
Be ready to answer questions of the type:
what’s the difference between a class and an object?
what’s the difference between inheritance and polymorphism?
In exercise-type interviews, the interviewer will ask you to open a text editor to solve one or two exercises to test algorithms and data structures. Have a development environment ready!
In homework interviews, the interviewer will ask you to solve a medium-size coding challenge in a couple of hours or even days.
Use Git and write good commit messages. Follow best practices and code as clean as possible! It’s time to show off your coding skills.
Dont’ forget to add a README file to your solution. Include features you implemented, libraries you used, major design decisions you made and how to install your solution.
Now you know the interview types, it’s time to move on to the interview preparation.
Check the date and time. Make sure you have the time in your own timezone. Don’t show up the wrong time.
Install and setup the software needed. Is it Skype, Zoom or Hangouts? Did you receive an invitation link? Do you need a password to use the link?
Do not answer the videocall using your cellphone. Do you remember the type of interviews? It isn’t that comfortable to solve an exercise using the phone. You will inspire more trust turning on your camera.
Use video only if the interviewer uses it. Online etiquette.
Reduce background noise. Do not answer the videocall from a cafe, a shopping mall or a public hotspot. If you are staying at home, ask the people around you not to disturb you while in the interview.
Show up 10 minutes earlier. Don’t be late.
Be ready to introduce yourself using an elevator pitch. You will have less than 30 seconds to say who you are and what you have done professionally. Make sure to include in your pitch:
Years of experience
Education
Type of projects you have worked with: banking, marketing, etc
Be precise. Answer within 60 seconds. Don’t talk too much.
Summarize all your skills and strengths, before finishing the meeting. Give a good last impression.
Final tips
Voila! Now, you know the type of interviews you will find. Be prepared for them! Practice your elevator pitch and your project descriptions. Have a printed copy of them next to you during the interview, in case of panic attack. Be enthusiast and precise.
Here you are in another interview. You start to feel confident from your last interview. You have already evaluated postfix expressions and solved the two-number sum problem. Now, the interviewer challenges you with a new exercise: How to shift arrays elements to the right. Let’s solve it.
Given an array of integers and an integer k, rotate all elements k positions to the right. For example: After rotating [1,2,3,4,5,6] two positions to the right is [5,6,1,2,3,4]
Obivous solution: loop and bound checking
Your first approach is to roll a loop through the array and put in a second array each element shifted to the right.
You have to take care of elements near the end of the array. Otherwise, you will get outside of the array and an exception will be thrown. So, you add an if to check you keep it inside the bounds.
You can do better. Can you remove the bound checking?–the interviewer says.
Now, you start to use an example. If array=[1,2,3,4,5,6], k=1 and i=5, the last element must be the first one and so on and so forth. It reminds you the modulus operator (%).
The modulus operator, instead of dividing two numbers, calculates the remainder of dividing those two numbers.
Since the remainder is less than the divisor, you will always be inside the size of the array, if you use the modulus with the array length. So, you modify your previous solution.
What is the space complexity of this solution?–the interviewer asks.
Space complexity is a metric to compare the amount of memory required to run an algorithm in relation to its input size. If the input gets bigger, how much storage the algorithm requires?
Since you are using a temporay array, the storage will be proportional to the size of the array. So, it’s linear!
Right!–the interviewer replies. Can you come up with a constant solution?–he suggests.
You have to get rid of the temporary array! What if you shift one element at a time and the repeat the process as many times as needed? This solution isn’t the most performant, but it uses constant space.
This time you have to shift the array backwards to only keep one element of the array in a temporary variable.