Code Reviews Aren't Out to Get You.

-

I was recently put on a project to upgrade an application that was written by one of the other developers a number of years ago. As I started going through his code, it reminded me how important code reviews are for a development team. This particular developer is no longer with the company, and when he did work in our group, we didn't have solid procedures and standards for code development. We tried to do code reviews with this particular developer, but he was really obstinate, and only wanted to show demos of the application. Any time we asked to see code he would show us as little as possible. He also never checked code into the code repository, so when he finally left the company, I had to get access to his dev environment and check-in all his projects. 

Over the years I have seen this type of behavior often, and I still see it a little today. Fortunately, with solutions like GIT and TFS, there is a lot more collaboration built into the development environments. Back when we were using Visual Source Safe, our solutions weren't collaborative at all. When we switched to Team Foundation Server and converted to using GIT for all of our code, I was able to implement rules that alerted all the developers of a pull request. If more than one developer is working on a project, then at least one other developer has to approve the code before it gets merged into master. Even if a developer is working on a project by themselves, all developers are alerted of pull requests, so they can view the code. 

I still have issues getting developers to commit changes to their branches regularly, but that is getting better. I think the reluctance to embrace transparency is caused by insecurity and fear of criticism. I think we all suffer from imposter syndrome to some degree and if someone looks at our code they might discover we are frauds. Most developers have worked with an arrogant programmer at one point in their career who would point out every flaw as a way to boost his or her own ego and that leads to more insecurities in future projects.

The point of a code review is to make sure the code that is being posted to production is the highest quality possible. Having another set of eyes on your code adds another perspective and often another developer has found a better solution. That shouldn't be taken as a criticism but as an opportunity to expand your skills. I know that even the best code I write today when looking at it again six months from now will be flawed because I am constantly evolving as a developer. Having collaboration and advice from peers helps accelerate that evolution. 

The other side of the code reviews is seeing if there are any cool hacks that are being used. I love looking at other people's code and picking up new tricks on how to accomplish different tasks. In one of my recent projects, the other developer added a feature that captured a right-click event from the mouse in JavaScript and loaded a modal on the screen to select items to be added to a text area. It was a really cool feature, and something I wouldn't have thought of. Since then I have incorporated that same code in a number of my applications. 

My current project is going to be a lot of work because everything has to be rewritten. Even the database will need to be redesigned and the data migrated from the previous version. Had there been any transparency with this codebase originally, this application wouldn't be such a problem, and the developer would have improved his skills. Fortunately, we are learning from the past and our team is getting more and more collaborative, so projects that we are doing now will be much easier to update in the future.  

Comments

Post a Comment

Popular posts from this blog

Asp.Net Core with Extended Identity and Jwt Auth Walkthrough

-
Image
  Quite often I think of an idea for an app where I would have a website and a mobile app that share the same functionality. Every time I start to prototype something like this I get stuck on authentication. When you build a web application in Asp.Net Core, it is really easy to set up the built-in Identity platform to allow users to login, but if you want to allow a mobile app to access APIs on the same site, that takes a lot more work. Of course, you can use a cloud-based authentication system like AzureAd or Firebase authentication, but if you arent housing any sensitive data, that's a lot of extra plumbing that takes away from building your application.  This walkthrough will show you how to create an Asp.Net core site with Asp.Net Core Identity for authentication and authorization. We will be extending the identity to include additional user information and roles. We will also add jwt authentication and authorization, so SPA's and mobile apps can use the API's and share...
Read more

File Backups to Dropbox with PowerShell

-
Image
I have had a number of instances where I had a virtual server somewhere, and I wanted to have a backup strategy, but the paid plans were overkill for what I needed. I have all my source code in Github, so everything can be rebuilt. I just need to be able to archive database backups and uploaded files. I decided to use a Dropbox account and set up a scheduled task that calls a PowerShell script that accesses the Dropbox web API. Setting up Dropbox You will need to have a Dropbox subscription set up and then you can go to the developer's section at the following link: {% embed url=" https://www.dropbox.com/developers " %} On the main page, select create apps. On the following page, you will have the option to select the scope of the app. Here you can decide to only access a single folder or give access to all of the Dropbox functions. Either option will work, in my example, I used the full access scope, so I would have the option of putting different file types in different...
Read more

Dynamic Expression Builder with EF Core

-
I was recently working on an ASP.NET Core MVC project where the end user wanted to have a search form with several fields with drop down boxes and a couple of free-form text fields. One of the requirements was to be able to put "*" at the front or the end of a search term in the text boxes to indicate a wildcard search. That was the way a previous application worked and they wanted to maintain the same functionality. Since I was working with Entity Framework Core using a generic repository pattern, I wanted to be able to dynamically build the filter expression that I passed to the repository. I created a static Expression builder class with static methods for creating each type of expression needed. public static class ExpressionBuilder { public static Expression GetStringExpression ( string dataModelProperty , string dtoProperty , Expression combinedExpression , ParameterExpression parameterExpression ) { if ( dtoProperty...
Read more