Export (0) Print
Expand All
1 out of 1 rated this helpful - Rate this topic

Code Review for Application Blocks

Retired Content

This content is outdated and is no longer being maintained. It is provided as a courtesy for individuals who are still using these technologies. This page may contain URLs that were valid when originally published, but now link to sites or pages that no longer exist.

 

patterns & practices Developer Center

Microsoft Corporation

January 2005

Summary: How to code review application blocks using coding standards covering functional, deployment, performance, scalability, security, and globalization considerations.

Contents

Objective

Overview

Application Block Code Review

Tools

Summary

Objective

  • Learn the process for reviewing the implementation of application blocks, including various parameters such as performance, security, exception management, and globalization and localization features.

Overview

The implementation of application blocks should be validated against the various coding standards and best practices in the language that is used for developing the application block. The code should be reviewed from different perspectives, such as adherence to design and functional specifications, to ensure that it follows best practices related to parameters, including performance, security, globalization and localization, and exception management.

The examples in this chapter assume a design review of an application block named Configuration Management Application Block (CMAB). The requirements for the CMAB are the following:

  • It provides the functionality to read and store configuration information transparently in a persistent storage medium. The storage mediums are SQL Server, the registry, and an XML file.
  • It provides a configurable option to store the information in encrypted form and plain text using XML notation.
  • It can be used with desktop applications and Web applications that are deployed in a Web farm.
  • It caches configuration information in memory to reduce cross-process communication, such as reading from any persistent medium. This reduces the response time of the request for any configuration information. The expiration and scavenging mechanism for the data that is cached in memory is similar to the cron algorithm in UNIX.
  • It can store and return data from various locales and cultures without any loss of data integrity.

Application Block Code Review

This section describes the steps involved in performing a code review for an application block.

Input

The following input is required for a code review:

  • Requirements (use cases, functional specifications, deployment scenarios, and security-related requirements for the target deployments)

Design-related documents (architecture diagrams and class interaction diagrams)

Code Review Steps

The process for an application block code review is shown in Figure 5.1.

Ff649506.f05mtf01(en-us,PandP.10).gif

Figure 5.1. The code review process for application blocks

As shown in Figure 5.1, application block code review involves the following steps:

  1. Create test plans. Create test plans that list all test cases and execution details from a code review perspective.
  2. Ensure that the implementation is in accordance with the design. The implementation should adhere to the design decided on in the architecture and design phase.
  3. Ensure that naming standards are followed. The naming standards for assemblies, namespaces, classes, methods, and variables should be in accordance with the guidelines specified for the Microsoft® .NET Framework.
  4. Ensure that commenting standards are followed. The comments in the implementation should adhere to the standards for the language used for developing the application block.
  5. Ensure that performance and scalability guidelines are followed. The code should follow the implementation best practices for .NET Framework. This optimizes performance and scalability.
  6. Ensure that guidelines for writing secure code are followed. The code should follow the implementation best practices. This results in hack-resistant code.
  7. Ensure that globalization-related guidelines are followed. The code should follow globalization-related best practices in such a way that the application block can be easily localized for different locales.
  8. Validate exception handling in the code. The goal of exception handling should be to provide useful information to end users and administrators. This minimizes unnecessary exceptions at the same time.
  9. Identify the scenarios for more testing. During the white box testing phase, identify the scenarios that need more testing.

The next sections describe each of these steps.

Step 1: Create Test Plans

Create detailed test plans and detailed test cases for code review from various perspectives, including performance, security, and exception management.

Tables 5.1, 5.2, and 5.3 show the detailed test plan for the CMAB from the code review perspective.

Table 5.1: Sample Test case from the detailed test plan document for the CMAB

Scenario 1To ensure the functional mapping toward the design specification document.
PriorityHigh
Comments 
1.1LowTo ensure the functional mapping toward the design specification document.

Table 5.2: Sample Test case from the detailed test plan document for the CMAB

Scenario 2To use various tools against the application block.
PriorityHigh
Comments 
2.1HighTo validate the adherence to various programming practices by using FxCop to analyze various assemblies.
2.4HighTo verify that there are no politically incorrect instances in the comments by running an automated tool against the application block assemblies.

Table 5.3: Sample Test case from the detailed test plan document for the CMAB

Scenario 3To review the code of the application block using various guidelines prescribed by Microsoft for .NET development.
PriorityHigh
Comments 
3.1HighTo verify that the CMAB code follows the best practices and considers the trade-offs for better performance and scalability.
3.2HighTo verify that the CMAB code follows the best practices and trade-offs for security features.
3.3HighTo verify that the CMAB code considers the best practices for globalization.
3.4HighTo verify that the CMAB code follows the best practices for exception management.

For more information about how to create test plans for the application block, see Chapter 3, "Testing Process for Application Blocks."

Step 2: Ensure That the Implementation Is in Accordance with the Design

In typical development cycles, design precedes implementation. Developers write the source code in accordance with the design and the functional specification documents. The design and functional specification documents include the following:

  • High-level design documents. These include documents that show the component-level view of the application block. The component-level view contains the specifications about the classes in the components, the public methods (along with their signatures) in the classes and the description of the all components, the classes, and the public methods. The high level design document may also contain the description and flow of individual use cases.
  • Low-level design documents. These include documents that detail information such as the class-level variables, some pseudo code to illustrate the algorithm for the functions within the class, entity relationship diagrams showing the relationship between various classes and components, and sequence diagrams showing the process flow for various use cases.
  • List of assumptions and constraints. This list includes assumptions, design standards, system constraints, and implementation considerations.

During code review, you must ensure that the application block implementation is in accordance with the design and functional specifications. It is preferable to translate the design, requirements, and the functional specification documents into checklists. During code review, review these checklists to ensure that each of the classes and functions is in accordance with the specifications for the application block. The code should be modularized in a way that avoids code repetition for the same functionality.

For example, if you are doing the code review of the CMAB, you should watch out for the following:

  • The code should maintain a design of inheritable interfaces to handle the different persistent mediums that are to be used to save the configuration settings.
  • The code should consist of modules that specifically address every persistent medium that is listed in the functional specification document.
  • The setting options in the configuration file for enabling and disabling caching in memory information should be implemented using parameterized methods. This ensures that a minimal amount of code can effectively address all the options and their implementations with appropriate options for scalability.

Step 3: Ensure That Naming Standards Are Followed

A good naming style is one of the parameters that help the code to be consistent and identifiable. It allows quick walkthrough and easy understanding of the code, thereby helping redress the problems in the code structure. This helps achieve process-dependent maintainability instead of people-dependent maintainability. Any developer can easily understand the functionality from the functional specifications and read through the code to see the actual implementation. This helps when a developer other than the original author fixes bugs or adds additional functionality.

Some common points that need to be handled during a code review from the naming conventions perspective are the following:

  1. Capitalization styles. Classes, events, properties, methods, and namespaces should use Pascal casing; whereas parameter names for functions should use Camel casing. Pascal casing requires the first letter in the identifier and the first letter of each subsequent concatenated word to be capitalized. For example, the class should be declared as shown in the following code.
    //It is recommended to name a class like this
    public ImplementClass
      

    The casing in the following code is not recommended for naming a class.

    //It is not recommended to name a class like this
    public implementclass
      

    Camel casing requires the first letter of an identifier to be lowercase. For example, a function that takes in parameters should have parameters declared as shown in the following code.

    void MyFunc(string firstName, string lastName)
      

    The casing in the following code is not recommended for the function arguments.

    //It is not recommended to name the function arguments like this
    void MyFunc(string FirstName, string LastName)
      
  2. Case sensitivity. Check if all the code components are fully usable from both languages that are case-sensitive and languages that are not case-sensitive. Just because a particular language is case-insensitive, the naming conventions should not be a mix of different cases. Also, the identifier names should not differ by case alone. For example, you should avoid using the following declarations because a case-insensitive language cannot differentiate between them.
    string YourName 
    string yourname
      
  3. Acronyms and abbreviations. Well known and commonly used acronyms should be used in the code. However, the identifier names should not be made into abbreviations or parameter names as shown in the following code.
    //Use this declaration
    void ValidateUserIdAndPwd()
    //Instead of this
    void ValUIAP()
      

    You can use Camel casing or Pascal casing for the identifiers and parameters that have acronyms that are longer than two characters. However, you should capitalize the acronyms that consist of only two characters as shown in the following code.

    //Use this declaration
    MyWebService.RA
    //Instead of this
    MyWebService.Ra
      
  4. Selection of words. The words used for identifiers and parameter names should not be the same as standard words or key words in the .NET Framework class library.

For detailed information about naming guidelines and related checklists, see "Naming Guidelines" in the.NET Framework General Reference on MSDN at http://msdn.microsoft.com/library/default.asp?url=/library/en-us/cpgenref/html/cpconNamingGuidelines.asp.

Step 4: Ensure That Commenting Standards Are Followed

Commenting the code is one of the most important practices that significantly increases the maintainability and usability of the application block. The developer can read through the comments to understand and customize the code for the application block to suit the application needs.

Commenting the code is important, but it should not be redundant. A block of code that follows other coding standards does not need to have comments for every line of code and will be readily understandable even without extensive commenting. However, comments are important to give the necessary information about the purpose or methodology used by the logic. Some of the factors that need to be handled while doing the code review from the commenting standards perspective are the following:

  • Make sure that all methods, classes, and namespaces have all the proper information such as comment headers, clearly specifying the purpose, parameters, exception handling, and part of the logic (if required) for that method.
  • Make sure that there are comments for complex logic in all methods. Also make sure that these comments clearly explain the logic and its complexity. If required, you can use an example in some cases to help explain the logic.
  • Make sure that there are appropriate comments to indicate why a particular variable is assigned a particular initial value. For example, if any integer variable is being initialized with a certain number, such as 0 or 1, the comment should clearly specify the relevance of the initialization.
  • Make sure that you spell check comments and that proper grammar and punctuation is used.
  • Make sure that the comments do not use politically incorrect words that can be considered as biased.
  • Make sure you use language-specific styles for commenting the code.

Microsoft Visual C#® development tool allows you to comment inline within an identifier, such as a namespace, class, or function, using the C style syntax as shown in the following code.

//This is a single line comment
/*This is a block comment 
  spanning multiple lines */
  

The language also allows you to add documentation comments in XML notation as shown in the following code.

/// <summary>
/// This class is used for reading the configuration...
/// </summary>
  

These types of comments usually precede a user-defined type, such as a class or interface. You can compile a documentation file for these comments using a documentation generator. The following are some of the main commenting guidelines for C#:

  • Avoid block comments for descriptions; you should use documentation style comments for description of the identifiers.
  • Single line comments are preferred when commenting a section of code.
  • The length of comment should not significantly exceed the length of code it explains.

For Microsoft Visual Basic® .NET development system, the recommended approach is to use the (') symbol instead of using the REM keyword because it requires less space and memory.

For more information about guidelines for documentation comments in C#, see "B. Documentation comments" on MSDN at:
http://msdn.microsoft.com/library/default.asp?url=/library/en-us/csspec/html/vclrfCSharpSpec_B.asp.

For more information about commenting guidelines for Visual Basic .NET, see "Comments in Code" on MSDN at:
http://msdn.microsoft.com/library/default.asp?url=/library/en-us/vbcn7/html/vaconCommentsInCode.asp.

Step 5: Ensure That Performance and Scalability Guidelines Are Followed

The code review ensures that the implementation adheres to best design practices and implementation tips that help improve performance and scalability. Some of the important things you should look for in any source code are the following:

  • Code ensures efficient resource management. From a performance perspective, the first and foremost step in the code review is to validate that the code handles its resources efficiently. The performance of the application block significantly depends on the way code engages and releases resources. If the code holds on to a scarce resource for longer than required, it may result in behaviors such as increased contention higher response times or increased resource utilization. The following are some of the guidelines for efficient resource management:
    1. The resources should be released as early as possible. This is especially true in cases of shared resources, such as database connections or file handles. As much as possible, the code should be using the Dispose (or Close) pattern on disposable resources. For example, if the application block is accessing an unmanaged resource, such as a network connection or a database connection across client calls, it provides clients a suitable mechanism for deterministic cleanup by implementing the Dispose pattern. The following code sample shows the Dispose pattern.
      public sealed class MyClass: IDisposable
      {
          // Variable to track if Dispose has been called
          private bool disposed = false;
          // Implement the IDisposable.Dispose() method
          public void Dispose(){
          // Check if Dispose has already been called 
          if (!disposed)
          {
      // Call the overridden Dispose method that contains common cleanup 
      // code
          // Pass true to indicate that it is called from Dispose
          Dispose(true);
      // Prevent subsequent finalization of this object. This is not 
      // needed because managed and unmanaged resources have been 
      // explicitly released
          GC.SuppressFinalize(this);
          }
          }
      
          // Implement a finalizer by using destructor style syntax
          ~MyClass() {
      // Call the overridden Dispose method that contains common cleanup 
      // code
          // Pass false to indicate the it is not called from Dispose
          Dispose(false);
          }
      
          // Implement the override Dispose method that will contain common
          // cleanup functionality
          protected virtual void Dispose(bool disposing){
          if(disposing){
          // Dispose time code
          . . .
          }
          // Finalize time code
          . . .
          }
          ...
      }
        
    2. The code should use try-finally blocks in Visual Basic .NET or statements in C# to ensure resources are released, even in the case of an exception. The following code sample shows the usage of a using statement.
      using( StreamReader myFile = new StreamReader("C:\\ReadMe.Txt")){
          string contents = myFile.ReadToEnd();
          //... use the contents of the file
       
      } // dispose is called and the StreamReader's resources released
        
    3. The classes accessing the unmanaged object should be separated out as wrapper classes for the object without referencing to other unmanaged objects. In this way, only the wrapper object is marked for finalization without other leaf objects being promoted into the finalization queue for an expensive finalization process.
    4. Analyze the class and structure design and identify those that contain many references to other objects. These result in complex object graphs at run time, which can be expensive for performance. Identify opportunities to simplify these structures.
    5. If you do not require a class level member variable after the call, check before making a long-running I/O call. If this is the case, set it to null before making the call. This enables those objects to be garbage collected while the I/O call is executing.
    6. Identify the instances where your code caches objects to see if there is an opportunity to use weak references. Weak references are suitable for medium-sized to large-sized objects stored in a collection. They are not appropriate for very small objects. By using weak references, cached objects can be resurrected easily, if needed, or they can be released by garbage collection when there is memory pressure. Using weak references is just one way of implementing caching policy. For more information about caching, see the "Caching" section of "Chapter 3—Design Guidelines for Application Performance" in Improving .NET Application Performance and Scalability on MSDN at http://msdn.microsoft.com/library/default.asp?url=/library/en-us/dnpag/html/scalenetchapt03.asp.
    7. Ensure that the code does not call GC.Collect explicitly because the garbage collector is self-tuning; therefore, programmatically forcing garbage collection is more likely to hinder performance than to help it.
    8. Investigate where the buffers (required mostly for I/O calls) are allocated in your code. You can reduce heap fragmentation if they are allocated when the application block is initialized at the application startup. You should also consider reusing and pooling the buffers for efficiency.
    9. Consider using a for loop instead of foreach to increase performance for iterating through .NET Framework collections that can be indexed with an integer.
  • Code performs string operations efficiently. String operations in managed code look deceptively simple, but they can be a cause of memory overhead if they are not handled efficiently. The reason behind this is that the strings are immutable. If you concatenate (or perform some other operation that changes the string itself) a string to another string, the old object has to be discarded with the new value being stored in a new object. To minimize the affect on these side effect allocations, especially in loops, you should ensure that the following guidelines are followed:
    1. StringBuilder is used when the number of concatenations is unknown, such as in loops or multiple iterations. Concatenation using the + operator may result in a lot of side effect allocations that are discarded after each new iteration. The usage of StringBuilder is shown in the following code.
      StringBuilder sb = new StringBuilder();
      Array arrOfStrings = GetStrings();
      for(int i=0; i<10; i++){
          sb.Append(arrOfStrings.GetValue(i));
      }
        
    2. If you know the number of appends and concatenate strings in a single statement or operation, give preference to the + operator.
    3. Watch out for code that calls the ToLower method. Converting strings to lowercase and then comparing them involves temporary string allocations. This can be very expensive, especially when comparing strings inside a loop. You should give preference to using the String.Compare method. If you need to perform case-insensitive string comparisons, you can use the overloaded String.Compare method shown in the following code.
      // The last argument is to signify a case-sensitive or case-//insensitive comparison
      String.Compare (string strA, string strB, bool ignoreCase);
        

    The CMAB performs a number of string operations because it handles a lot of XML manipulation. Even after a thorough review of the code, it is important to ensure that there are no side effect allocations taking place. You can mark the various use cases for white box testing in such a way that they can be profiled using the CLR profiler.

    For more information about CLR profiler, see "How To: Use CLR Profiler" in Improving .NET Application Performance and Scalability on MSDN at http://msdn.microsoft.com/library/default.asp?url=/library/en-us/dnpag/html/scalenethowto13.asp.

  • Code uses efficient data structures. The data structures used in the code minimize the overhead of enumeration and provide appropriate functionality for searching and sorting. For example, if you are enumerating through the value types, you should implement the enumerator pattern for a custom collection to avoid the overhead of boxing or unboxing.

For more information, see the following resources:

Step 6: Ensure That Guidelines for Writing Secure Code Are Followed

Security code reviews help you identify areas in your code that do not follow the best practices for writing secure code. Identifying this code is important because it directly weakens the security in one way or other. The security vulnerabilities that get introduced because of they do not adhere to guidelines for writing secure code are difficult to catch during black box testing and may surface later as a threat in a production scenario.

The following are some of the vulnerabilities you can look for during security code reviews:

  1. Check for hard coded information. There should not be any hard coded information, such as UserName, Password, or ConnectionString, in any portion of the source code. This information should be kept in a configuration file.
  2. Code reviews should ensure that the code uses code access security appropriately, as discussed in the following examples:
    • During the code review, make sure that the privileged code identified during the design and build phase requests appropriate permissions while accessing secure resources or while performing privileged operations.
    • Review that the permission requests are appropriate and minimum. For example, if your code needs to read a specific key in the registry, it should request read permission to that specific key in the registry.
    • Check whether the assembly is strong named; make sure that it does not use the AllowPartiallyTrustedCallersAttribute (APTCA). In cases where it is must use APTCA, make sure that the appropriate code access security demands are used.
  3. Check for a cross-site scripting attack. Never place too much trust on the data entered by the user; always validate it. Use regular expressions to validate data in ASP.NET applications.
  4. Check for buffer overflows. Validate length and data type of parameters and calls to the unmanaged code (including Win32 DLLs and COM objects) through COM interop or P/Invoke layers.
  5. Disable detail error messages and tracing. Check whether the detailed error messages are disabled from popping up on the client computer and tracing is disabled for the ASP.NET applications.
  6. View state protection. Check whether the view state is enabled only when required. Also check that view state protection is enabled as applicable.

Security code reviews can also help you identify test scenarios for white box testing. For example, if your code handles input data, you can make sure during security code review that the input data is validated. Security code review can also help to identify white box testing cases by reviewing the logic and validations that have been done.

For more information about security code reviews, see "Chapter 21—Code Review" in Improving Web Application Security: Threats and Countermeasures on MSDN at:
http://msdn.microsoft.com/library/default.asp?url=/library/en-us/dnnetsec/html/THCMCh21.asp.

Step 7: Ensure That Globalization-related Guidelines Are Followed

Globalization is the process of ensuring that a piece of software supports localized versions of the user interface, such as messages shown to the user (such as exceptions, warnings, and information) and that it can operate on regional data (such as different date notations and currency formatting).

Before starting the code review, you should note all cultures the application block will be supporting. The common issues related to globalization features revolve mostly around a set of features. This includes features such as any type of string operations, including comparing, sorting, or indexing, formatting date and time in specific cultures, using calendars in specific cultures, or comparing and sorting data in specific cultures.

The following are some of the best practices you should review for the application blocks:

  • Preferably, the application should use Unicode internally. It is preferable for the application block to use Unicode encoding, though other techniques, such as DBCS (Double-Byte Character System), are also available. If you plan to use a database, you should use Unicode-compatible data types for database design. For example, the CMAB stores configuration information in a SQL Server database. This information can be from different character sets; therefore, you should make sure that the database design uses data types such as nchar, nvarchar, and ntext.
  • Ensure that the culture-aware classes provided by the System.Globalization namespace are used to manipulate and format data. The various rules for languages and countries, such as number formats, currency symbols, and sort orders, are aggregated into a number of standard cultures. In the .NET Framework, culture is represented programmatically using the System.Globalization.CultureInfo class.

    The .NET Framework uses a combination of two cultures to handle different aspects of localization, namely Current culture and Current user interface culture. Current culture determines how various data types are formatted, such as numbers and dates. Current user interface culture determines which localized resource file the resource manager loads. The code may set the culture in the configuration file as shown in the following code.

    <configuration>
        <system.web>
        <globalization culture="en-US" uiCulture="de-DE"/>
        </system.web>
    </configuration>
      

    The code may also set the culture programmatically as shown in the following code.

    using System.Globalization;
    using System.Threading;
    
    Thread.CurrentThread.CurrentCulture = 
        CultureInfo.CreateSpecificCulture ("de");
    
    // Set the user interface culture to the browser's accept language
    Thread.CurrentThread.CurrentUICulture = 
        Thread.CurrentThread.CurrentCulture;
      
  • For sorting, check if the application block uses the SortKey class and the CompareInfo class.
    // Creates a SortKey using the en-US culture.
    CompareInfo myComp_enUS = new CultureInfo("en-US",false).CompareInfo;
    SortKey mySK1 = myComp_enUS.GetSortKey( "llama" );
      
  • For string comparisons, the CompareInfo class should be used.
    // Defines the strings to compare.
    String myStr1 = "This is one string";
    String myStr2 = "This is another string";
    
    // Creates a CompareInfo that uses the InvariantCulture.
    CompareInfo myComp = CultureInfo.InvariantCulture.CompareInfo;
    ConsoleWriteLine(" The result of case insensitive comparison is: " + myComp.Compare( myStr1, myStr2, CompareOptions.IgnoreCase));
      
  • Formatting should be done using the appropriate classes, such as the DateTimeFormatInfo class for date and time formatting and the NumberFormatInfo class for numeric formatting. The following code shows an example of using DateTimeFormatInfo.
    DateTimeFormatInfo dfmt = Thread.CurrentThread.CurrentCulture.DateTimeFormat;
    dfmt.LongDatePattern = @"\[yyyy-MMMM-dd\]";
    string longDateString = DateTime.Now.ToLongDateString(); 
      
  • If you need to convert from a string to a DateTime data type, you should use the DateTime.Parse method as shown in the following code.
    DateTime dt = DateTime.Parse(aFrenchDateString, new CultureInfo("de")); 
      
  • If your application block is to be integrated with a Web application that can be accessed from different time zones, you should use the universal time feature provided by the .NET Framework. Use the universal time feature for internal storage of the configuration values that can be converted into local time zone values upon retrieval. Use the DateTime.ToUniversalTime and DateTime.ToLocalTime methods for performing these conversions.
    DateTime localDateTime = System.DateTime.Now;
    System.DateTime univDateTime = localDateTime.ToUniversalTime();
      
  • Check if the application block can read and write data to and from a variety of encodings by using the encoding classes in the System.Text namespace. Ensure that the code does not assume ASCII data and that it assumes international characters will be supplied anywhere a user can enter text. For example, in the CMAB, international characters should be accepted in server names, directories, file names, and so on. You can use the static GetEncoding method of the System.Text.Encoding class to retrieve an Encoding object for a specific Code-Page, and then use its GetBytes method to convert Unicode strings to a Code-Page specific byte array.
    Encoding iso = Encoding.GetEncoding("iso8859-1");
    Encoding unicode = Encoding.UTF8;
    byte[] unicodeBytes = unicode.GetBytes(src);
    iso.GetString(unicodeBytes);
      
  • Whenever possible, check if strings are handled as entire strings instead of as a series of individual characters. This is especially important when sorting or searching for substrings. This will prevent problems associated with parsing combined characters.
  • The text should be displayed using the classes provided by the System.Drawing namespace.
  • For consistency across operating systems, make sure that the code does not allow user settings to override CultureInfo. The code should use the CultureInfo constructor that accepts a useUserOverride parameter and set it to false. This is most useful when using application blocks in an application that is not Web based. For Web-based applications, the effective settings are that of the ASP.NET process.
  • If a security decision is based on the result of a string comparison or case change operation, check if the code is performing a culture-insensitive operation by explicitly specifying the CultureInfo.InvariantCulture property. This practice ensures that the result is not affected by the value of CultureInfo.CurrentCulture.

For more information, see the following resources:

Step 8: Validate Exception Handling in the Code

Robust exception handling is a very important feature that significantly affects the user experience with the application. It helps administrators managing an application and it helps developers to analyze problems that surface in the actual production environment. Exception handling is an important design feature. During the design phase, you capture all the possible exceptions that need to be handled for a particular use case. The code review makes sure that the code appropriately handles the exceptions. The following are some guidelines for exception handling that should be reviewed in the code:

  1. Exception handling should not be used to control the application logic. It should be used only in exceptional cases because throwing exceptions is expensive. For example, if the CMAB is checking for configuration information from the database for a particular key and does not find any value, this is an expected condition and is not a fit case for throwing exception as shown in the following code.
    // Bad scenario to throw exception
    static void ConfigExists( string KeyForConfig) {
        //... search for Key
        if ( dr.Read(KeyForConfig) ==0 ) // no record found
        {
        throw( new Exception("Config Not found"));
        }
    }  
      

    These conditions should be handled by returning a simple Boolean value. If the Boolean value is true, the configuration information exists in the database.

    static void ConfigExists( string KeyForConfig) {
        //... search for key
        if ( dr.Read(KeyForConfig) ==0 ) // no record found 
    {
        return false;
        }
        . . .
    }
      

    Returning error information using an enumerated type instead of throwing an exception is another commonly used programming technique in performance-critical code paths and methods.

  2. All exception conditions identified for a particular use case should be handled. You should make sure the exceptions are not caught only to be rethrown to the user because throwing exceptions is expensive. It should be done only if you are adding some value or performing some action on the exception object, such as logging into an event sink.
    try {
        //try to load an XML file
    LoadXmlFile(xmlApplicationDocument, _applicationDocumentPath);
        }
        catch (XmlException)
        {
    //Log the exception before re throwing it
    LogExceptionInSink(XmlException)
    //if the loading of the document fails throw a configuration 
    //exception stating that configuration document is invalid
    throw new 
    ConfigurationException(Resource.ResourceManager["RES_ExceptionInvalidConfigurationDocument"]);
        } 
    finally
    {
        // Code that gets run always, whether or not
        // an exception was thrown. This is usually
        // clean up code that should be executed
        // regardless of whether an exception has
        // been thrown.
    }
      
  3. Code does not swallow exceptions or inefficient code that catches, wraps, and rethrows exceptions for no valid reason. If you do not need to act on a particular exception, you should allow the exception to bubble up from the application block code to the client application. The user integrating the application block can capture the exception and decide on how to act on the exception.
  4. Make sure that the finally blocks are used to ensure that the resources are cleaned up. Make sure of this even in the case of exception as shown in the following code.
    SqlConnection conn = new SqlConnection("...");
    try {
        conn.Open();
        // Some operation that might cause an exception
    
        // Calling Close as early as possible
        conn.Close();
        // ... other potentially long operations
    
    }
    finally {
        if (conn.State==ConnectionState.Open)
    conn.Close();  // ensure that the connection is closed
    }
      

    You can also use the using statement in C# to clean up resources that require a call to Dispose ()method for deterministic clean up of resources.

  5. Preferably, the logging mechanism should be configurable in such a way that the client can decide the event sinks for logging of particular type of errors. For example, the default configuration of the CMAB logs high severity errors in the event log and logs all the warnings and informational messages in Windows Event Trace.

    The configuration should also provide an option of switching on and off the logging either partially or completely. This is because logging might be an undesirable overhead in the production environment.

  6. Check whether the exceptions caught by any part of the program or subprogram are propagated through the system and necessary actions are taken, depending on the exception type. Functions should avoid returning error codes to the calling functions; instead, an exception propagation mechanism should be used.
  7. Make sure that the code uses custom exceptions. The .NET platform gives provision for creating custom exceptions. Custom exceptions generally help in giving more detailed information on the exception that occurred or in highlighting any business logic discrepancies. For example, if the CMAB fails to read the configuration file during the application initialization process, it throws an exception of type ConfigurationException or a class derived from ConfigurationException.
  8. Check whether the unhandled exceptions are properly managed. For example, in ASP.NET applications, the application's Web.config file can be used to show a custom error page for the clients instead of showing the exact stack trace of the error to the clients by the following settings.
    <customErrors defaultredirect="http://hostname/error.aspx" mode="on">   
    </customErrors>
      
  9. Exception management should also ensure that any informational messages are also logged. These events are useful in monitoring the health of the application block. For example, the CMAB can report any requests that hold locks for writing or updating information for more than 3 milliseconds.

For more information about exception management architecture, see Exception Management Architecture Guide on MSDN at:
http://msdn.microsoft.com/library/default.asp?url=/library/en-us/dnbda/html/exceptdotnet.asp.

Step 9: Identify the Scenarios for More Testing

During the code review process, testers get to review the code extensively through various perspectives, such as performance, security, and globalization. During this review, testers make sure the code is in accordance with the guidelines; however, this is not a substitute for actually testing the scenario. The goal of the review is to minimize surprises in the later stages.

During the code review, testers should always try to identify the usage scenarios for additional testing. These are the scenarios that require extensive testing rather than a simple code review, such as in the following examples:

  • You have done a code review of an internal function of a component that takes numeric input. You would want to make sure that during white box testing, the function is thoroughly tested for all types of inputs such as 0, -ve numbers, numbers out of the accepted range, and decimals.
  • During the code review, you come across a function that makes remote calls. You may want to profile the time taken by the function when the block is under a concurrent load of users. If the remote call is an I/O call for retrieving information in buffers, you may want to further load test it with different payloads to avoid the possible problems of buffer overflow.
  • A particular function does the sorting of string data before returning the values in an array. This function needs additional testing for various cultures that have been specified in the requirements.

The scenarios identified during code review are tested more in the black box testing and white box testing stages, such as performance testing and globalization testing.

Tools

The FxCop tool can be used to check for compliance of code with various types of guidelines. The tool analyzes binary assemblies (not source code) to ensure that they conform to the .NET Framework Design Guidelines, available as rules in FxCop.

The tool comes with a predefined set of rules, but you can customize and extend them.

For more information, see the following resources:

Summary

This chapter presented a process for reviewing the implementation of an application block. A thorough code review ensures that you can significantly cut down on the time spent during white box testing. The erring code that has been caught during the code review of the modules can be fixed very easily but if it slips through and is caught during the white box testing, it may result in cascading changes across the modules.

Start | Previous | Next

patterns & practices Developer Center

Retired Content

This content is outdated and is no longer being maintained. It is provided as a courtesy for individuals who are still using these technologies. This page may contain URLs that were valid when originally published, but now link to sites or pages that no longer exist.

Did you find this helpful?
(1500 characters remaining)
Thank you for your feedback
Show:
© 2014 Microsoft. All rights reserved.