Saturday, November 14, 2015

The Importance of Design, Architecture, and Clean Code in a Startup Environment



I've worked for several startups, and practically all suffer from the same basic problem: money. It is a race against time to develop enough customers and/or revenue before investor interest, and thus money, disappears. But to get customers, you need code...usually lots of it. So, naturally the focus is on pumping it out as rapidly as possible. The catch-22 is that practically as soon as the company starts to turn the corner and become profitable that code can turn into a liability. Often companies pass the critical early phases, only to fail at scale because of short-sighted decisions in getting to that first corner.

For emphasis, I will give one extreme example I encountered at a very small startup many years ago where I was managing engineering. I inherited a hastily written code base as a starting point (developed by people who were engineers, but not software engineers). The lack of focus and interest in good code and development practices is perhaps best exemplified by the words of one of the co-founders that I will never forget. In a status meeting attended by the development team I  suggested that a developer at least extract a chunk of code that was "cut-and-paste" repeated into a single method. The Co-founder was at this meeting and interjected that this should not be done "if it might slowdown" the developer. This input, and even the notion that extracting code into a method would slow someone down, was a real eye opener. This was the most extreme case I've seen, and largely influenced by the fact that both the developer and the co-founder were hardware engineers, not software engineers, by background,  but the point is that sometimes even the most basic design and supportability tenets are disregarded in the flawed assumption that this is somehow justified in a startup environment.

In this case (as with most startups) the company did not survive long enough to become profitable, but what if it did? Would the weight of the technical debt sink the company? How could such a product be supported? Would there be the time and resources available to completely rewrite the code or would the company simply get bogged down in never-ending feature enhancements on a foundation that was already crumbling under its own weight?

So while some would argue that good practices is a luxury in startup environments, I would argue that the cost of completely disregarding good practices altogether assures failure. Clearly time is of concern, but "well-written" code need not necessarily take significantly more time than a complete hack. Some basic coding concepts can be applied with little or no additional time, including:

Beyond basic coding principles, it is equally important to spend time to design and architect the solution and at least consider the evolution of the system. I'm a firm believer that design can be broken down into iterations, similar to other coding activities. Start with the basic architecture required for the first milestone and map out where you expect it will need to evolve from there, expecting that the future iterations will change as requirements change. I'm not advocating for gold-plated designs, but you need to understand some basics of where the code will evolve to avoid coding into corners. I think you should fully understand the architecture requirements of the first iteration, and perhaps the next. For iterations beyond that, at least devoting some time to think about how well the existing architecture will scale can avoid pitfalls today that will be very costly tomorrow. I think this work should always be done in collaboration with other senior members and stakeholders of the team and should be supplemented with some form of "light" documentation. A day or two of due diligence can pay off dramatically over the lifetime of the project.

Saturday, October 17, 2015

Delivering Highly Configurable Libraries Using Dependency Injection



In the last post, I described usage of a factory pattern to provide highly configurable functionality. An alternate approach would be the usage of dependency injection(DI). At the time I designed these libraries, dependency injection platforms on Android were not as mature as they are today. Guice was starting to gain some popularity, but uses reflection and can impact runtime performance. With the advent of Dagger2, we have access to a static dependency injection solution that does not rely on run-time binding. 

Using Dagger2, I could replace the previous abstract factory pattern. I decided to explore this solution as an exercise. Note that I did not actually implement and compile this solution and humans are notoriously lousy compilers, so it's possible (likely) there are errors. However, it should illustrate the general approach.  This also assumes that you are generally familiar with usage of Dagger2.



As commented in the code. To override this behavior, the user could provide their own graph and data module and call ApplicationDispatchHandlerDaggerSample.registerHandlers() at runtime.

While some might argue that the DI solution with Dagger2 is cleaner, it would impose an additional constraint on each user of the NAC API to download and learn its usage. This is a relatively minor cost, but you could make the argument that in the interest of keeping NAC as easy to use as possible, the Dagger2 approach would add complexity with no real end-user benefit. A bigger issue with this approach is that, since we don't know which modules the user will want to  inject a-priori, we inject them all. This means that a user would have to specify handlers from within the NAC library, which  the user should not even need to know about, and which we have been keeping private and obfuscated. There are probably ways we could fix this by exposing methods to get the default implementation for a class, but this seems to just add more unintentional complexity. My conclusion is that, while DI is an important and highly useful pattern, for  this  specific use-case, the pattern we chose was an overall better approach.

Saturday, August 29, 2015

Enumerated Types - Java

Coming from a long C++ background, one of my favorite constructs in Java was the simple enumerated type. I found the added full-fledged, "class-like" behavioral characteristics useful from the beginning. Just the simple ability to specify a string value in the element constructors alone almost doubles the power of a C++-like enum because it allows you to essentially make String enumerated types when coupled with a toString() method overload. Of course there is much more power to the Java enumerated type's class-like capabilities than just this. One of the first really interesting usages I found for an enum was to create a simple state machine. I no longer have the source code, but if you are interested in how to do this web search for it, there are others who have done this.

When creating API's I find the bounded nature of enums to be self-documenting when used as parameters such as keys in key-value pairs, as opposed to traditional usage of strings. Additionally it removes the possibility of bugs from a consumer of the API making a typo in a string key. However, what if you are releasing an API that has both a defined set of key values and the possibility for unknown key values. I find that sometimes you have the need for an "extensible" enumerated type. For example, in one case we were delivering an Android library for usage in client software with a server-side back-end component that could add functionality over time. I wanted to provide the capability for a customer to use new keys delivered via new server functionality even though the library itself was unaware of these keys at the time of the library creation. However, I still wanted the self-documenting, less error-prone way of specifying keys provided by an enumerated type. To get both behaviors, we introduced a simple construct that we called DynamicEnum. A DynamicEnum is a class, not a Java enum, but uses String constants to enumerate the known values.

The DynamicEnum thus has the benefit of providing a set of values for all known element types, and can also be extended by adding additional values. In an upcoming post on usage of the abstract factory pattern I will describe why this can be useful through a real-world example.

Tuesday, August 18, 2015

Use Case: Being a Code Custodian, Manual Inspections



This is the third and final post of this series. Please see the last two posts for my thoughts on what it means to be a code custodian and the application of automated inspection tools in this role.

While automated tests have a place in detecting potential bugs or maintenance issues for a code-base, I find that only manual inspection can detect issues in proper usage of the classes, methods, and design patterns in use for a particular code base. At a recent positions, some of the stylistic things that I looked for in a manual inspection relate to reducing accidental complexity, and include:
  • Crisp and clear interfaces, especially customer-facing API's
    • Well-documented API calls
      • In this case, we used Javadoc
      • Many of the developers were non-native English speakers, making this more critical for our team
    •  Well-named methods
  • Proper usage of existing design patterns within the code base
  • Avoidance of internal packages
    • In our case, we had libraries that had external and internal packages. Sometimes developers would accidentally reach down into an internal package of another library
    • A better approach would have been to find an automated approach for verifying compliance
  • Proper application of standard object-oriented practices
  • Stylistic consistency
    • At a gross-level, without being intrusive
Of course, I also checked for more substantive issues like potential threading issues and other defects, but often the stylistic issues are more noticeable.

To avoid getting behind, I would review checkins on a daily basis on average. With about 10 active developers on the team, it wasn't practical to review every line of code that was checked in. To limit the time commitment I applied a few filters to guide my engagement level:
  • Individual developer expertise
    • Does this a developer have a track record of committing high-quality software?
    • Does this developer have experience in the area of the code that is being changed?
  • Type of code under change
    • Is this a core or performance-sensitive area of the code, requiring more attention?
    • Are the modules under change complex and/or already in need of refactoring
  • Customer-facing
    • Is this a customer-facing API or an internal method that will be obfuscated away?
We had a policy whereby developers were encouraged to ask for reviews of changes to critical and/or complex sections of code. The best developers in the group used this policy judiciously, and I would seldom review checkins by them unless they requested a review.  On the flip-side, if a developer had a track record of sloppy checkins and/or was working in an area of the code with which s/he had little familiarity I was likely to take more notice.

Having a role of technical lead, does not make anyone the expert on everything. I interpret the code custodian role to mean that I should coordinate with other experts on the team to help identify areas of risk and coordinate collaboration, not try to be the sole authority on everything. It's always helpful to get another set of eyes on your code, and I would frequently solicit code reviews for trickier areas of my own code submissions from one or two other senior members of the team. Additionally, I would sometimes ask for help from these same members in performing code reviews of submissions from others on the team that I find to be in a critical portion of the code that I have less familiarity with.

Of course it is very helpful to be intimately familiar with the software before taking on these types of reviews. In this case I had designed and coded much of the initial code base, which made the job easier. Were I to drop into a new code base, I could still look for general code and styling issues, but would be less apt to give any valuable feedback relevant to fitting into existing design patterns and proper usage of libraries, etc.

In an effort to preserve developer freedom, if the issues I detected were all small or stylistic, often I wouldn't say anything, but rather lock it away in case the advice might come in handy in the future as part of more substantive feedback. If the issue is in naming of methods and/or javadoc and the user is not a native-English speaker I might make the change directly in the code base and check it in since the changes are non-controversial and straightforward. For most other types of feedback I would usually make suggestions in an email. Developer woud either follow the suggestions or provide good reasons for not following them. Lastly, I find it can be helpful to reference well-known books by experts in the field when making a point to give it more credence and make it less of a single person's opinion.

In this series of posts I discussed the role of a code custodian in maintaining a clean base. I stress that this is not always an appropriate role and there are many different approaches and techniques. I don't necessarily advocate for the approaches laid out here, but list them by way of an example that worked well in one environment. I found the manual and automated inspections to be helpful but perhaps they would be too intrusive or unnecessary in another environment. In a third environment, perhaps more stringent techniques would be appropriate.

Saturday, August 8, 2015

Use Case: Being a Code Custodian, Automated Inspections



As stated in my last post, the roles of an architect and/or team lead can vary dramatically depending on team dynamics. Being a code custodian may or may not be appropriate for your organization. On the one hand, it's hard to argue against the importance of having a cohesive code base with similar coding patterns and standards for quality. On the other hand, a heavy-handed environment can be very disabling for developers and reduce team members' feelings of empowerment. So how best to balance these competing factors?

A recent long-running project I worked on had the following team dynamics:


  • Varied developer skill levels
    • Fresh from university to senior
  • Combination of remote and local developers
    • Most remote
    • English skills that also varied in quality between team members
  • Mix of time working in the code base
Although we had some written coding guidelines, I find this to be a largely ineffective way of managing a code-base. We did have some standards, but the best standards are already available and published in books such as "Effective Java" by Joshua Bloch and "Clean Code" by Robert Martin.

Instead, I chose to rely on a mix of automated and manual inspection approaches. See my last post for a comparison between these approaches.

Perhaps the most important tenet to developing any of these strategies is to get active participation from other members of the team in their usage and application. Some of the inspections by these tools can feel intrusive and even arbitrary at times. Getting the team or a subset of senior developers on the team to buy-in to the list of checks is important to foster a feeling of ownership to the policy and to avoid tying developers down and hampering creativity. it might take a couple of iterations to refine the rules.

For this situation, we used CheckStyle, FindBugs, and lint in out suite of tools that were run on Jenkins. Checking stylistic rules can be particularly treacherous if applied too heavy-handedly. When looking at CheckStyle, the first thing I did was to immediately drop silly rules such as where semicolons or spaces are placed. There is nothing worse than breaking the build because you missed a spacing constraint. I think that it's important to maintain consistency in the code-base, especially within an individual module. But this consistency does not need to be enforced via an automated tool.

However, that doesn't make the likes of CheckStyle worthless. There are a number of checks within CheckStyle that, when enforced, aid in keeping the complexity down in the code base. We used a number of CheckStyle rules, but the following were somewhat helpful in keeping code complexity manageable:

  • FileLength
  • MethodLength
  • ParameterNumber
  • AnonInnerLength
  • AvoidNestedBlocks
  • MagicNumber
  • Nested* (various Nested rules)
Some of the other methods, like HiddenField, were also useful in avoiding bugs.

FindBugs and lint (and occasionally CheckStyle) were good at identifying potential defects and leaks. At first the tools detected a number of issues, but over time there were fewer and fewer violations.

Having good reporting mechanisms was very important. At first our reporting mechanisms were very poor, requiring a lot of digging to find the source of the build error. This was largely due to the limitations of the central build CI system we were using. When we shifted the tests to run on Jenkins, the reporting was much crisper, making for a much better experience.


Friday, August 7, 2015

An Extensible Factory Pattern - Use Case



NuanceAndroidCore (NAC) was designed to be a highly customizable library, facilitating creation of Siri-like voice assistants, without imposing constraints on the look and feel of these assistants. The platform controls the voice dialog and provides callbacks to the UI layer for displaying recognition results and status of the recording and recognition. Consumers of the library render this feedback as they see fit.

The library also performs data lookups such as calendar events and alarms within a range of dates; and actions such as launching an application, adding/deleting/changing alarms, events, and playing music. Much of the benefit of the library comes from its ability to perform all this seamlessly and automatically. However, in many cases, individual customers might have their own implementations for one or more of these applications, with their own API's, and sometimes with added functionality. The challenge was to come up with the best way to support these customizations, control and manage the intricacies of the voice dialog, and supply default behaviors for everything else.

This is where the extensible factory proved to be useful. NAC used this pattern in several places, including creation of its action and lookup handlers. NAC registered its own implementations of each handler (for example a handler for sending an SMS message). Each customer could then customize specific actions by registering a different factory to override the default implementation, should they desire different behavior. In this manner, they could leverage everything else the library provides, including everything related to the voice dialog for that action, and any actions that didn't need customizations, and only provide code for specific overrides of the default behavior.

Each of the action handler factories are chosen from a map keyed off an element of the server response (for example, an action of type "sms" would key an action handler of type SMS). Building off the sample code introduced in the last post about dynamic enums, the factory method looks like:


(Disclaimer: This snippet was hand-edited from the original code and not compiled. The possibility exists for compilation errors.) 

This technique relies on reflection to build up the handlers, but performance is not a concern because the construction of the handlers is a very small portion of the overall time to perform an action, typically dominated by the time in getting recognition results returned from the server.

The voice recognition logic  and dialog management for this system is server-side.
So the opportunity exists for new voice domains to be added and deployed to the field that NAC is not configured for, after NAC has already been released. This is where the dynamic enum class discussed in the last post became useful. With the dynamic enum, NAC could register all its known handlers with immutable keys, while also supporting key types it doesn't know about. For example, the SMS key type would be defined and used by a customer wishing to override default SMS handling, but the customer could create a new key, say "sports" on the fly should the "sports" domain be added to the server domains after the NAC library has already been released.

In the next post, I will explore an alternate approach using Dagger2 for dependency injection.


Saturday, July 25, 2015

Being a Code Custodian



The roles of a team lead can vary dramatically depending on the team dynamics. Being a code custodian may or may not be appropriate for your organization. I have found it valuable in some situations. There are automated and manual approaches to monitoring the integrity of software. I like to apply both when appropriate. Each has pros and cons:

  • Automated Approach - Usage of software tools to detect potential defects and/or style violations
    • Advantages
      • Can check for a large number of potential defects and/or design smells
      • No ongoing time investment after initial investment to setup and configure
      • Can be tied into CI environments like Jenkins to guarantee adherance
    • Disadvantages
      • Not good at catching the most important issues
      • If rules are configured too restrictively, becomes more of a hindrance than a benefit
  • Manual Approach - Human review of checkins or some subset of checkins
    • Advantages
      • Only way to catch many types of issues
      • Keeps the reviewer abreast of new development
      • Leads to mentoring opportunities for the reviewer
      • Good way for the reviewer to learn from other approaches and pickup new techniques
    • Disadvantages
      • Difficult and important to be completely objective and remove opinion from feedback
      • If applied heavy-handedly 
        • Kills developer empowerment
        • Can be disastrous to morale
      • Time-intensive
      • Spotty
        • Usually unrealistic to fully check everything
I would further decompose automated approaches into 2 categories.
  • Category 1: Surface Analysis - Static-analysis tools including lint, checkstyle, and findbugs
    • Advantages
      • Customizable
      • Rules-based
      • File-driven
      • Easily integrated into many modern IDE's
      • Easily integrated into build or CI environments via a CLI
      • Many free or open-source
      • Often mature and robust
    • Disadvantage
      • While they can catch many "surface" issues, they don't always dig very deeply
  • Category 2: Deep Analysis - Tools that looks more deeply into aspects such as cyclomatic complexity, memory usage, and performance. 
    • Advantage
      • Tend to be higher value 
    • Disadvantages
      • Difficult to automate
        • Some intended more as an aid for manual test and debug
        • Some produce results that should be inspected and interpreted
      • Can be difficult or impossible to quantify rules for
      • Less typically free or open-source

In the next two posts I will delve into how I applied these tools in recent projects.