Ensuring quality code in large-scale projects

When I came to Airbnb in the fall of 2012, there was, to put it mildly, there was some confusion and vacillation. Some time ago, the company began to grow and develop at a tremendous pace. In the first place, this was expressed in traffic and transaction volumes. To cope with all this, very quickly increased the staff of developers. A year before my arrival, there were 16 people in the group, I had 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 increasing 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 overlapped with each other. We had to solve many difficult problems before, but in general they were connected with:

• scaling whole applications on Ruby on Rails, which were created without any calculation for subsequent scalability,
• and an increase in the development team to solve the previous problem.
')
It is now clear that we have managed to cope with numerous problems of growth. And here I would like to talk about how we proceeded to writing a more maintainable, more technological code. And, as a result, much more suitable for scaling.

Now I work in a group that develops a module for processing payments . Therefore, for us, the quality of the code, its stability and maintainability are of critical importance. Given the volume of transactions, even small errors are unacceptable. Over the past year, we have done a great job in developing rules and techniques that allow us to write code quickly and at the same time qualitatively. First of all, we are talking about establishing and observing specific styles and conditions when coding, regular verification of code by colleagues (peer review) and testing.

Code consistency

The consistency of the code depends on the syntactic style and compliance with the agreements and techniques when writing. You have read and heard about it many times, but I’ll repeat it: 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 positive effect on our work.

In a general sense, computer code is consumed by two classes of entities: machines and people. The machines don't care what the code looks like, as long as it compiles without problems. But developers are not all the same. If your team uses different styles and approaches to writing and solving problems in the code base, this leads to an excessive cognitive load on all participants, and a long understanding of the code that is not created by you. If all members of a 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 have to 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 team members.

In Airbnb, we improved the consistency of the code for two years. We first developed a style guide (there were several versions of this guide). The most detailed tutorials on JavaScript and Ruby , also available on RSpec , API design, service design, etc. Despite the fact that these guidelines are based on our specific requirements and conditions, many of them are now used by other teams as well as individual developers. Of course, the potential benefits of these guides depend 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 to place the opening brace. These and other moments - 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 strings are not only more readable, but also allow you to define statements in a simpler form (especially when they are used with descriptive variable names). Methods consisting of a series of short simple operators are easier to comprehend and modify. Even diffs as a result more accurately show what has changed between commits. And since we are writing a 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 } 

Suppose we need to change the register. 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 thinking what exactly has changed, without parsing the string in my 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 differential 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 such an approach, at first faced with the resistance of colleagues. I had to show perseverance, and as a result it became a habit for all of us - to make short lines. Very extreme examples on this topic can be seen in the manuals of Joint Strike Fighter C ++ and JPL C Coding . Obviously, these standards are redundant for most consumer web applications, so always correlate the project level and goals that you want to achieve by adhering to certain rules. It is important to keep a balance.

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

Code verification by colleagues

Regular audit of the code of their colleagues has become the second cornerstone of increasing code consistency. The most important advantage of this process is that it allows you to quickly and effectively detect all sorts of bugs before they get into the next release. But there is a cross-audit and many other small positive points. Over the past year we have been auditing for every change made to the code.

We try to involve in the audit everyone who is somehow involved in this part of the code or functionality. As a result, for 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 learn things previously unknown to themselves (for example, credit card processing, internal database organization, cryptography, etc.). If the change in the code involves a 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 solutions are rolled out. All this is a good school for beginners, by the way.

It is important to remember that the mere existence 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 from the point of view of mutual aid, but is also an excellent 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 contributes to their involvement and improves the quality of work.

Here is an example of a situation where the main role is not played 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 can lead to a catastrophic drop in performance. Adding a select to the User.active object means that Rails will access MySQL, invoke and instantiate all active users, put them into an array, iterates it and selects users whose country corresponds to RUS. And all this only to count them.

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

Here is another example:

badly:

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

OK:

 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 summarize 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 burden on MySQL, we do not generate unnecessary network traffic and avoid potentially huge computations in Ruby.

The above examples demonstrate how we improve the responsiveness of our site and even prevent it from falling. Peer review is also good because by discussing a specific part of the code, employees periodically raise a variety of topics, such as database structure or generating financial reports. That is, lively and productive dialogues constantly arise around the code, during which we study and teach others.

Testing

I will not go into this question, this is very well written in one of our blog posts . I can only say (and it is unlikely to be something new for you) that testing is an incredibly important process in terms of providing high quality, maintainable code. However, the point is not 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 member of the team. When writing tests is a habit, then creating testable code also becomes a habit.

Conclusion

To summarize what was said.

All members of a professional development team, no matter what size 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 code by other people. The best start is to create coding style guides. Even deceptively simple and obvious things can greatly affect the quality of the code, its stability and the simplicity of refactoring.

It is necessary to actively and constructively review each other’s code. At least one person, besides the author, must look through all the diffs. The benevolent submission of proposals and their verification of the code is a prerequisite for team building and growth of the professional level. This helps beginners to learn faster and more fully, to 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 to compromise on this or “cut the road”. The more we test, the more it turns into a habit. As a result, you can even love this process.