Android Code Quality: Structure
Posted on 02/05/2014 by Dr. Nils Göde
Code quality audits are a fundamental part of our daily work as software quality consultants. The objective of such an audit is to assess the quality of a system’s source code and identify trends based on the code’s evolution. That makes an audit an important prerequisite for ongoing quality control and improvement. Understandably, we are not able to publish the results of the audits that we do for our customers. That makes it sometimes hard for us to explain what such an audit looks like. Instead of lengthy descriptions, we decided to analyze the code quality of an open-source system and use this as a showcase to illustrate what our quality audits look like.
There are different categories of quality aspects that we analyze in our code audits. This is the first of a series of posts—each of which will deal with a single category. This first post introduces the system and focuses on its code structure.
We chose the source code of Android as subject for our analysis. Android is a well-known operating system for mobile devices with freely accessible source code. Android is based on the Linux kernel, written mainly in C/C++, and development is driven by Google. While we often analyze much larger systems, focusing on the central component
platform/system/core helps to keep this post brief. Among other things, the core contains basic operating-system functionality, a basic graphic library (
libpixelflinger), the Android Debugging Bridge (
adb), and a selection of utilities. We obtained the code from the corresponding Git repository. The tool we used for our audit is Teamscale. We connected the Git repository to Teamscale which then analyzed each relevant commit in the repository. A commit is relevant if it changed at least one file containing C or C++ code. In total, Teamscale analyzed 1,344 commits that were made since October 2008.
The first thing we look at is the size of the codebase. As of January 8th, the core component contains 149,128 lines of code (LOC). Excluding blank lines and lines that contain only comments (SLOC), the size is 102,971. The size on its own does not tell us anything about the quality of the code. However, it still gives us a first impression of the system and helps us when assessing quality attributes. The following screenshot taken from Teamscale shows how the size evolved since 2008. The vertical blue lines are baselines that we configured in Teamscale. We selected some prominent releases of Android that are from left to right the versions 2.0 (Eclair), 2.3 (Gingerbread), 4.0.1. (Ice Cream Sandwich), 4.3 (Jelly Bean), and 4.4.1 (KitKat).
The figure shows a notable growth of the codebase during the last years with some major changes. The growth in August 2010 was caused by adding the MirBSD Korn Shell mksh. The decrease in June 2011 was caused by moving mksh out of the core again and to another component. The decline in November 2013 was caused by the removal of the ash shell. The subsequent increase in the same month was caused by adding a number of utilities.
The structure and organization of the source code is relevant when it comes to maintaining the code. A good modularization with reasonably sized and named components saves a lot of time when locating relevant code. And what people are often not aware of: A significant share of the total maintenance effort goes into locating and searching code.
Directories, Files, and Namespaces
A first indication for poor structure is a lack of directory hierarchy—for example, all source files are contained in a single directory. Modularizing the system by putting associated files in a corresponding directory is a first step towards better structure. When looking at Android, we can see that the source code files are already organized in meaningful directories that resemble the different components of the system. The naming of files and directories is consistent and follows the usual C/C++ practice. This is good and there are no negative consequences for future maintenance. The C++ code of the system is wrapped in a dedicated namespace
android which is also an indicator for good structure.
Apart from being in an adequate directory, each source code file should not contain too much code. With too much code in a single file it becomes hard to locate the relevant functions inside. In addition, other activities that are done on a per-file basis (for example reviews or unit tests) become also more expensive if files are large. With respect to object-oriented systems, files often correspond to classes and large classes are unlikely to implement a single concern. Based on our experience we suggest that files should be generally kept shorter than 400 LOC. We are aware that this might not be possible in some situations and exceptions can be tolerated. However, there is no reason for files to be larger than 1,000 LOC. These thresholds may need some adjustment based on the corresponding project context but we consider them to be a reasonable choice in general.
We first categorize each file based on its maintenance risk. Short files have low risk (green), medium-sized files have medium risk (yellow), and long files have high risk (red). Our overall assessment is then based on the percentage of code lines that are contained in low-risk, medium-risk, and high-risk files respectively.
The figure above shows the percentage of code that is in files with less than 400 LOC (green), in files between 400 and 1,000 LOC (yellow), and above 1,000 LOC (red). First thing to notice is that less than half of the code is in the green area and more than 18% of the code is in the red area. That means there are quite a number of large files that developers come across during maintenance. The largest file is
libpixelflinger/scanline.cpp with 2,363 LOC closely followed by
adb/sysdeps_win32.c with 2,220 LOC.
The figure above shows an excerpt from our CQSE benchmark comparing the file size in Android to other systems we have analyzed before. The same configuration was used for the analysis to make the results comparable. The systems A to D are systems from our customers. The figure shows that the file size in Android is better compared to most other C/C++ systems we have analyzed. Still there is some room for improvement since C/C++ systems tend to have long files in general.
One of the central entities in a C/C++ system are the functions in the code. Each function should perform a single conceptual task. In consequence, functions should be kept short to make the one task they do easily comprehensible. Short functions do not only save effort for program understanding, they also reduce the risk of code clones. While long functions can hardly ever be reused in their totality, individual parts that are needed somewhere else are often duplicated with copy and paste. Since clones have a number of negative consequences, functions should be kept short to make them easy reusable. Long functions are also an obstacle to detailed profiling which is mostly done on the function level.
As with the file size it is hard to specify one fixed limit for all systems. As a rule of thumb, a function should not fill more than one screen, that is, a developer should be able to inspect it without scrolling up and down. Based on our experience we suggest that functions should not be longer than 40 LOC. Again, there may be a number of exceptions but an upper limit of 100 LOC should not be reached. Again, we classify each file based on the maintenance risk. A file in which all functions are below 40 LOC has low risk (green), files with at least one function above 40 LOC but no function above 100 LOC has medium risk (yellow), files that contain a function above 100 LOC have high risk (red). The categorization is once more done on a per-file basis because of the high likelihood that the long function has to be understood whenever the file is changed.
The figure above shows that the Android calls is almost evenly distributed across the three categories. That means that one third of the code is in files that contain at least one function longer than 100 LOC. The results indicate that develoeprs are often confronted with long functions and face the negative consequences mentioned above. In total, the Android source code contains 489 functions that are longer than 40 LOC. 95 of these functions are even longer than 100 LOC. The longest function is
adb/commandline.c (573 lines) followed by
toolbox/newfs_msdos.c (480 lines). The code shown in the figure below belongs to a single function in the code:
adb/commandline.c. While command-line processing is often implemented in a single function, we are not aware of any justified reason for this. Why should command-line processing not be implemented in a modular fashion making the addition and removal of parameters much simpler?
The figure below shows how the results compare to other systems. While the number of long functions is somewhere in the middle compared to other C/C++ systems, it is still very high. With respect to code comprehension and low redundancy, we suggest to improve the code by shortening long methods wherever possible.
Another obstacle for code comprehension is deeply nested code. Each nesting level opens up a new context with certain conditions that hold and that do not hold. The number of conditions that one has to keep in mind increases with every level of nesting. Therefore high-quality code has only a low level of nesting. We regard a nesting level of up to 3 (the function itself does not count) to have no negative consequences for the maintenance of the code. In contrast, a nesting level of more than 5 is very likely to increase the time needed to comprehend the code. Again, we have categorized each file based on its maintenance risk. The figure below shows that Android does notably better compared to file size and function size.
The maximum nesting depth found in Android is 8. Although we have seen much deeper nesting in other systems, this value is still too high. The figure below shows one of the statements with nesting level 8.The statement is contained in the function
find_usb_device in the file
adb/usb_linux.c. While it is comparatively easy to see that
link_len > 0, and
slash are true in this line, there are at least 5 other conditions that hold. Understanding the complete context takes quite some time. In addition, the example points out two other things. First, deep nesting often occurs in long functions (
find_usb_device has 170 lines) as a side-effect of bad modularization. Second, the comments at the closing braces are an often-seen cry for help and prove the poor comprehensibility of the code.
Once more, the Android code is somewhere in the middle considering the nesting across the whole system as the figure above shows. The nesting is probably not a major obstacle to the maintenance of the code but should be improved where possible. In many cases, this can be combined with breaking larger functions into smaller functions.
In summary, the structure of he Android code is slightly above average compared to other C and C++ systems. Still, there are many situations where the structure can be improved—primarily by reducing the size of functions. All in all, with respect to structure, the Android code is of medium quality. But as already mentioned in the introduction, structure is only one of multiple categories that we look at when determining code quality. So stay tuned for the continuation of this audit.