Code quality assurance in large-scale projects

Original author: John Terenzio
  • Transfer


When I came to Airbnb in the fall of 2012, there was, to put it mildly, some confusion and stagger. Some time ago, the company began to grow and develop at a tremendous pace. This was primarily expressed in the volume of traffic and transactions. To cope with all this, the staff of developers was also increased very quickly. The year before I joined the group, there were 16 people, with me there were about 40, and now over 130. And one of the main problems caused by all these processes was the preservation of the quality of the code in a rapidly growing and complicated project.

Looking back, it seems to me that this was a turning point in the history of our company. Explosive development has led to a number of technical and cultural challenges for Airbnb, which for the most part intersect with each other. We had to solve many difficult tasks before, but in general they were associated with:

• scaling up solid Ruby on Rails applications that were created without any expectation of further scalability,
• and expanding the development team to solve the previous problem.

It is now clear that we have managed to cope with numerous growth problems. And here I would like to talk about how we moved on to writing more convenient, maintainable, more technological code. And, as a result, much more suitable for scaling.

Now I work in a group developing a module for processing payments . Therefore, for us, the quality of the code, its stability and ease of maintenance are critical. Given the volume of transactions, even small errors are unacceptable. Over the past year, we have done a great job of developing rules and techniques that allow us to write code quickly and at the same time with high quality. First of all, we are talking about the establishment and observance of specific styles and conditions during coding, peer review of the code regularly and testing.

Code Consistency


Code consistency depends on the syntactic style and compliance with conventions and techniques when writing. You have read and heard about this many times, but I repeat: if you can open a file from your code base and determine by style which of your colleagues wrote it, then this is a very bad sign . The development of internal coding standards and their observance by all team members had an extremely favorable effect on our work.

In a general sense, computer code is consumed by two classes of entities: machines and people. All the same, the machines do not care how the code looks, if only it compiles without problems. But the developers do care. If your team uses different styles and approaches to writing and solving problems in the code base, then this leads to excessive cognitive load on all participants and a long understanding of the code that you didn’t create yourself. If all members of the group write code in a more or less similar manner, debugging is greatly simplified, it becomes much easier to maintain someone else's code. I do not mean that you should make a choice in favor of some decisions for the sake of readability or simplicity of the code, but there are enough ways to unify the coding process by all members of the team.

At Airbnb, we have been improving code consistency for two years. First, we developed a style guide (there were several versions of this guide). The most detailed JavaScript and Ruby guides are also available on RSpec , API design, service design, etc. Despite the fact that these guides are based on our specific requirements and conditions, many of them are now used by both other teams and single developers. Of course, the potential usefulness of these guides depends on the type of application you are working on, but if you are developing in JavaScript or Ruby, I recommend that you familiarize yourself with them anyway. At least for inspiration.

Some rules are accepted by us exclusively arbitrarily. For example, by indentation (tab and space), or on which line should the opening brace be placed. These and other points are a matter of taste. It is important that the rules are respected by all involved. On the other hand, in some cases, options are possible.

Take the length of the string. We prefer to make lines short and descriptive. Short lines are not only more readable, but also allow operators to be specified in a simpler form (especially when used with descriptive variable names). Methods consisting of a series of short simple operators are easier to read and modify. Even diffsas a result, they show more precisely what has changed between commits. And since we are writing testable code containing small, concise methods, this encourages the creation of a very “clean”, modular and easy to understand code base.

Consider an example:

usernames = Users.where(:status => 1, :deleted => false).map{ |u| u.first_name.downcase }.reject{ |n| n == ‘john’ }.map{ |n| n.titleize }


Let's say we need to change case. Diff will show something like this:

-  usernames = Users.where(:status => 1, :deleted => false).map{ |u| u.first_name.downcase }.reject{ |n| n == ‘john’ }.map{ |n| n.titleize }
+ usernames = Users.where(:status => 1,  :deleted => false).map{ |u| u.first_name.upcase }.reject{ |n| n == ‘JOHN’ }.map{ |n| n.titleize }


It is difficult to say without a hint what exactly has changed without steaming the line in your head. And if the code was originally written like this:

users = Users.where(:status => 1, :deleted => false)
usernames = users.
      map{ |user| user.first_name.downcase }.
      reject{ |name| name == ‘john’ }.
      map{ |name| name.titleize }


That difference in diff would be much clearer:

users = Users.where(:status => 1, :deleted => false)
usernames = users.
      - map{ |user| user.first_name.downcase }.
      - reject{ |name| name == ‘john’ }.
      + map{ |user| user.first_name.upcase }.
      + reject{ |name| name == ‘JOHN’ }.
      map{ |name| name.titleize }


It's funny, but initially those of us who promote this approach at first encountered the resistance of our colleagues. I had to be persistent, and as a result, it became a habit among us all to make short lines. More extreme examples on this subject can be seen in the Joint Strike Fighter C ++ and JPL C Coding guides . Obviously, these standards are redundant for most consumer web applications, so always correlate the level of the project and the goals that you want to achieve by observing certain rules. It is important to keep a balance.

As I mentioned, we have also created guides for developing APIs and services. Despite the fact that it is not always possible to clearly prescribe such things in advance, the consolidation of the team’s general knowledge into a single document has provided invaluable benefits.

Code verification by colleagues


Regular code audits by colleagues have become the second cornerstone of improving code consistency. The most important advantage of this process is that it allows you to quickly and efficiently detect all kinds of bugs before they get into the next release. But there are many other small positive aspects in the cross-audit. Over the past year, we have been conducting an audit for each change to the code.

We try to engage in the audit everyone who is somehow involved in this part of the code or functionality. As a result, according to the most non-trivial pull request, at least one or two independent discussions arise. Sometimes the parsing process goes so far that some have to study things previously unknown to themselves (for example, credit card processing, internal database organization, cryptography, etc.). If the change in the code touches on some big important issue, then we create the appropriate recommendations and documentation. I want to emphasize that when reviewing, we are not allowed to appeal to our status or position. The contribution of any member of the team is appreciated, and the best decisions are rolled out into the battle. All this is a good school for beginners, by the way.

It is important to remember that the mere presence of recommendations does not mean that they will be followed (or at least read), or that they are suitable for all occasions. But reviewing is not only very effective in terms of mutual assistance, but it is also a great way to learn or teach someone the art of programming, which is difficult to do with ordinary textbooks. And when people learn a lot at work, it promotes their involvement and improves the quality of work.

Here is an example of a situation where the main role is played not by the coding style, but by a wise approach:

rus_users = User.active.select{ |u| ‘RUS’ == u.country }
puts “There are #{rus_users.length} RUS users today”


and

rus_users = User.active.where(:country => ‘RUS’)
puts “There are #{rus_users.count} RUS users today”


The first option is more resource-intensive, and with large amounts of data it can lead to a catastrophic drop in performance. Adding selectto an object User.activemeans that Rails will access MySQL, call and instantiate all active users, put them in an array, iterate it, and select users whose country matches RUS. And all this is just to count them.

In the second example, we also start with the object User.active, but then apply the where filter. The first line does not initiate any database queries. When in the second line, when we request a counter, Rails only makes a request SELECT COUNT(*)and does not bother calling lines or instantiating models.

Here's another example:

bad:

amount = Payout.
where(:successful => true).
where(‘DATE(timestamp) = ?’, Date.today).
inject(0) { |sum, p| sum += p.amount }


well:

amount = Payout.
where(:successful => true).
where(‘DATE(timestamp) = ?’, Date.today).
sum(:amount)


In the first example, we limit the search to one day, but even in this case, you need to call and instantiate a large number of objects just to sum over one field. In the second example, MySQL deals with the summation during the search, which then simply returns the value we need. In general, this does not create additional load on MySQL, we do not generate unnecessary network traffic and avoid potentially huge calculations in Ruby.

The above examples demonstrate how we improve the responsiveness of our site and even prevent its crashes. Peer review is also good because, discussing a specific part of the code, employees periodically raise a wide variety of topics, such as database structures or generating financial reports. That is, lively and productive dialogs constantly arise around the code, during which we learn ourselves and teach others.

Testing


I will not delve into this issue, this is very well written in one of the posts of our blog. I can only say (and it is unlikely that this will be something new for you) that testing is an incredibly important process in terms of ensuring high quality code that is convenient to maintain. However, the point is not in the maximum coverage of the code with tests, the main thing is to create a testing culture so that it becomes a conditioned reflex for each team member. When writing tests is a habit, creating testable code also becomes a habit.

Conclusion


I will summarize all that has been said.

All members of a professional development team, no matter how large it is, must write code in the same, pre-approved style. This helps to widely use successful approaches and techniques, facilitates maintenance and maintenance of the code by other people. A better start would be to create coding style guides. Even deceptively simple and obvious things can greatly affect the quality of the code, its stability and ease of refactoring.

It is necessary to actively and constructively review each other's code. At least one person, in addition to the author, must view all the diffs. The friendly submission of proposals and their verification of the code is the key to team building and professional growth. This helps beginners learn faster and more fully, acquire useful skills and habits. In the end, it can save your application from a serious fall.

It is important to develop a strong testing culture, not compromise or cut the road. The more we test, the more it becomes a habit. As a result, you can even love this process.

Also popular now: