Code Commenting Reality

If you're a coder, you've heard it a thousand times: 

"Comment your code." 

You might even have worked in an environment that mandates javadoc comments on every function, or worse yet, failed to land a job because your code did not have "enough comments".

I used to be a docs and comments nut, years ago. I was one of the few coders that actually maintained my comments over time.

To get right to the point, experience has shown me that "commenting" is generally a knee-jerk default that is abused as a crutch under the guise of "accepted practice".

I'll let the book Clean Code: A Handbook of Agile Software Craftsmanship, do the talking here. If you're a coder and have never heard of this book...well, you want to do something about that. The chapters on commenting and formatting are very relevant to the new wave of "software metrics" (which personally I see as a way of trying to commoditize the art and practice of coding):

"There are certainly times when code makes a poor vehicle for explanation. Unfortunately, many programmers have taken this to mean that code is seldom, if ever, a good means for explanation. This is patently false."

"Usually [comments] are crutches or excuses for poor code or justifications for insufficient decisions."

"It is just plain silly to have a rule that says that every function must have a javadoc, or every variable must have a comment."

- Martin, Robert C. (2008-08-01). Clean Code: A Handbook of Agile Software Craftsmanship

I find this all to be absolutely true, and not "sort of, sometimes."

Here's the perfect example. I see this ALL the time regarding event handlers. I do mean ALL the time, at every single organization and contract I've worked for (look at my LinkedIn profile to get an idea of the scope of that statement):

function handleClick ( obj ) { ... }

This coder will be told, in a review or something, "what's it handle exactly?"

The coder will do this:

// This functional handles a click when the user clicks the Submit button
// It accepts the params from the form that the Submit button is part of.
function handleClick ( obj ) { ... }

Hey I did a good job. I added comments to my code to clarify it. Very professional.

Not really. You copped out on the thinking. It's like taking medication for something that could be prevented by exercise.

This is better, because it removes the need for the comment.

function onFormSubmitClick ( formArgs ) { ... }

Should you never comment anything?

Of course not. But a comment, should be considered a temporary compromise; the code is good enough, causes no defects, but isn't easily intelligible with a reasonable amount of perusal. I should probably rethink it when I get a chance, so that I can remove the need for the comment.

So if somebody says to you, "your code doesn't have enough comments," what this more than likely actually means, is that your code is difficult to understand without more comments. Which means it's probably not very well written. Try again.

My final, and favorite, quote from Clean Code:

"The proper use of comments is to compensate for our failure to express ourself in code. Note that I used the word failure. I meant it. Comments are always failures."

As always, thanks for visiting.

Code Review Reality

It's been a while since I posted. However, I've been more active in development than ever, and naturally have developed more opinions on certain aspects of coding. Call it a by product of experience and observation, coupled with a reporter's instincts (if you didn't know, I was a newspaper reporter in a past life).

One thing that I have latched onto, is code review.

I'll be blunt as to the reason for this research; I instinctively never bought the idea that ad-hoc code review is a meaningful way of identifying and reducing defects, and my research and observation up to this point indicates that code review is generally adopted based on anecdote, is usually implemented in a short-cut fashion ("desk inspections"), and as such, will have little to no real, consistently measurable result as far as reducing defects.

When you've finished derisively snorting and if you haven't already dismissed this blog post as ignorant, please continue reading.

As some background, over my 2.5 decades as a professional developer, here's how I've seen code review introduced at most organizations:

• A team of coders creates a product, very quickly, to get a prototype off the ground.
• That product begins to gain traction.
• "In the wild" bugs, become embarrassing, and sometimes costly.
• Management pushes engineering on the matter.
• Senior coders state that most problems are due to the prototype quality of the code.
• Senior coders request time to reengineer the tiers of the system.
• Management rejects this recommendation in favor of delivering selling-point features.
• Management insists on a process to measure bugs.
• Bug counting at phases of the development-release cycle is introduced.
• Management demands that the bugs at each phase of the cycle be drastically reduced or eliminated.
• Senior coders again state that the prototype nature of the software makes this very difficult.
• Management again rejects requests to reengineer.
• Hiring begins in earnest: "more coders means more productivity," (a well known fallacy).
• Hiring rates for senior level coders are disappointing.
• Management looks to things like "process improvement".
• Code Review, supposedly cheap, easy to implement, and a quality improvement "no brainer", is introduced.
• It doesn't get the anticipated results; bug patterns appear more or less the same over time.
• The data is pushed around, argued, determined to be insufficient to draw real conclusions.
• More processes are introduced (common reluctance to abandon "investment" in a failed system).
• Nothing really changes.
• The process is often perceived as ponderous and/or punitive by coders.

Regarding the latter, the litany of articles on how to try and avoid code review being perceived as punitive and ponderous, should make it clear that the sentiment is common. I won't bother citing references to such an easily predictable fact.

I'm sure many of you reading this have seen a similar pattern. Does it mean code review is pointless and ineffective?

Not at all. I want to be clear on this point; I am NOT saying that code review has no purpose.

What I am saying, is that short-cut "desk inspection" code review without the prerequisite processes to improve the code that is submitted for review in the first place, and without building in preparation time to conduct reviews properly, can reduce productivity and morale, and increase cost, without any real result in terms of reducing defects.

So if you are going to implement code review, be realistic about what can be achieved with it. If you are doing desk inspection of fragments, then no other result aside from improved formatting and use of internal standards, if the organization actually has such standards to refer to (and most do not), should be expected. If the organization does not have an internal standard to refer to (and again, most don't), then ad-hoc desk inspection code review is pointless.

To keep this blog post short of a long white paper, I'll present a handful of findings that correlate with my overall findings and observation.

Consider this statement from "Agile Testing and Quality Strategies: Discipline Over Rhetoric" by Scott Ambler and Associates. Note that he makes an important distinction regarding Agile vs. Serial development:


"First and foremost, I consider the holding of reviews and inspections to be a process smell which indicate that you have made a mistake earlier in the life cycle which could be very likely rectified by another, more effective strategy (non-solo development, for example) which has a much shorter feedback cycle. This goes against traditional wisdom which says that reviews are an effective quality technique, an opinion which is in fact backed up by research -- in the book Practical Software Metrics for Project Management and Process Improvement Robert Grady reports that project teams taking a serial (non-agile) approach that 50 to 75 percent of all design errors can be found through technical reviews. I have no doubt that this is true, but my point is that although reviews are effective quality techniques in non-agile situations that in agile situations there are often much better options available to you than reviews, so therefore you should strive to adopt those quality techniques instead."

There's another important point I infer from this. If you are finding lots of bug in your code reviews, then your coders need to up their game. An obvious bug caught in review should be quite rare. 

Let's talk about the cost of catching a bug in the coding cycle. I found this graph, taken from the previous source, to be pretty much dead on.

Note that defects found via review or inspection are more costly than defects found in a number of processes that should be implemented before code review, such as Pair Programming, Continuous Integration and Test Driven Development. If bugs are getting through to review...you have a problem that is costing you money.

Another interesting read was the IEEE white paper, "The Impact of Design and Code Reviews on Software Quality: An Empirical Study Based on PSP Data" by Chris F. Kemerer, Member, IEEE Computer Society, and Mark C. Paulk, Senior Member, IEEE.

This paper is related to much more formal and highly academic software development setups than your typical "agile" (I use the term loosely) startup or small development shop has in place (again, stressing the result between serial, and agile, development) But even when using such an environment as the control, it correlates that preparation prior to a review, such as familiarizing oneself with the overall system, requirements, and design, is crucial in achieving the effect of reduced defects.

"Despite consistent finding that inspections are generally effective, Glass has summarized the contradictory empirical results surrounding the factors that lead to effective inspections. Weller found that the preparation rate for an inspection, along with familiarity with the software product, were the two most important factors affecting inspection effectiveness."

So, if you are not intimately familiar with a given area of the software product, and do not have time to prepare for the review to get a complete understanding of the design and requirements, you should not be expected to conduct reviews that measurably decrease defects. Anything short of this discipline throws the value of code review and inspection, as far as reducing defects, into debate:

"In summary, although the benefits of inspection are widely acknowledged, based on these competing views and conflicting arguments the discipline has yet to fully understand the fundamental drivers of inspection costs and benefits."

Let's focus on "preparation" for a code review.

The faux-agile notion of code review is a quick "desk inspection": you get a fragment of code, look it over quickly, give it a pass, or open an issue for comment. There's a little back and forth, something might get tweaked (e.g. "we put returns before our brackets"), the coder eventually gets a thumbs up, and commits.

I reject the notion that enough meaningful and costly-to-fix bugs can and will be caught in such a review process to make it worthwhile. I found this correlated in numerous sources that were far more critical of the subject, but I decided to lead with the IEEE paper because it speaks of the power and value of code review...if it is done correctly. That means proper preparation, which means more time, which means more cost.

So if you are implementing code review and expecting it to be a cheap way of reducing defects...well, I hope your job doesn't depend on the results.

Here is one such more outspoken source.From "Code Review - The Meaningless Ritual" by Daniel Pietraru:

"Done properly code reviews are extremely time consuming. Today’s programmers are more efficient, due to tools and better languages, then 30 years ago. They can create more code faster. But the amount of code inspected per hour is about the same because they are not smarter than people 30 years ago. Imagine 1000 lines of code that one programmer can write in 1-2 days.

With a traditional code inspection 4 people are required to inspect this code at a rate of 200 LOC/hour after they prepared the code review by reading the code and understanding the design. If we are optimistic the whole thing will require 6-8 hours per developer. This means a total of 24-32 hours spent on inspecting code produced in 8-16 hours. Probably this kind of review can find a lot of bugs in the code but is it worth the price? How many teams get away with estimates including 2-3 hours of inspection for every hour of development?"

People might say those production numbers are unrealistic. So, I'll give real data. In one month, I pushed 84 commits, with 2000+ deletions and 5700+ changes (numbers from GitHub). So push those numbers out to a team of say, 15 coders. In fact, cut the number in half if you think I'm unusually productive. Then do the review-cost math submitted by Pietraru.

EXPENSIVE. And that's just the desk inspection version. With proper inspection, those reviews could have taken a week...just for one coder, me.

And to be frank, if I had to get through reviews of all my code, that productivity would have dropped significantly. There's stats for you; if I reduce my productivity to zero, I won't introduce any bugs, proving the value of reviews.

Pietraru also goes on to point out a very interesting fact; where did we get code reviews from at all? Again, numerous sources correlate the following:

"Code inspections or reviews as we know them come from the hardware world. Initially Michael Fagan [from IBM, 1976] used them for hardware design reviews. This fact is very significant because uncaught defects in hardware design are pretty expensive.

Also defects in low level software were very expensive at the time. Imagine you write a piece of code that has to be burned on a ROM chip (non erasable). You cannot afford to wait and test/debug your code until after the ROM was destroyed. But this is not the case anymore. Unit tests and running the code in debuggers for success and failure paths are pretty cheap with modern IDEs and testing frameworks."

This reinforces the notion stated by the "Agile Testing and Quality Strategies" paper cited before; in a shop that purports to be "agile":

"...you should strive to adopt those [other] quality techniques instead".

In closing:

There is simply too much information available to show that ad-hoc "desk-inspection" code reviews in and of themselves do not usefully correlate to a measurable reductions in defects. In its useful form, code review is a disciplined practice involving a significant investment of time, which translates to money. It met with success in hardware programming, because the investment was offset by the irretrievable cost of producing bad hardware. But this has nothing to do with most software development today; modern IDEs and productivity tools allow us to write FAR more code than hardware programmers did in the 70s, and we can push out hot patches, update UIs without swapping out server bits, and so forth. We're not worried about the cost of 100,000 bad ROMs.

Ultimately, such short-cut reviews can raise overall costs and have other negative effects. There are other more effective cultural changes and processes available that can really make a difference in the quality of your product.

Dealing with the MX namespace in the FX world


So one day, you decide you're not going to use an mx DataGrid, because you know it's just not necessary; all you want to do is display a list, and if you haven't heard (...if you haven't, why are you reading this article...), Spark components are all the rage. So you set up a stub project, use an s:List, set up an item renderer, fake some data, do a little style work, and it all works great. 


Then you try to integrate it into a Flex 3.5 project. You're already using the "both mx/fx" setting in the project properties, but there's a lot of files in there that were built before Spark. 


You can end up with all kind if problems:

•  If you have any interface implementations in your top-level element (usually a Canvas or some such), suddenly the interface methods will come up as "not implemented".
• Your style for the List won't work, any number of odd errors, like "mobile theme only" and some such appear. 
• Namespaces will be called undeclared.
• "Only one language may be specified" sorts of errors. 

Here's what you need to make sure of:

• Declare your top-level mx and fx namespaces this way (you'll also need the "s" namespace for Spark components):

xmlns:fx="http://ns.adobe.com/mxml/2009"
xmlns:mx="library://ns.adobe.com/flex/mx"
xmlns:s="library://ns.adobe.com/flex/spark"

• If you have inline style problems, this is an example of making it work. Note that I'm styling all s:List components with this, and even though the "s" namespace is declared above, you'll still need it below in the style declaration.

<fx:style>
@namespace s "library://ns.adobe.com/flex/spark";
s|List { baseColor: #e5e5e5; }
</fx:style>

• And the big one...make sure you change the namespace of your script tags if you are seeing problems with interface implementations and whatnot.

<fx:script></fx:script>

That should do it, these things have solved most of problems when dealing with the mx namespace in the fx world. 

As always, thanks for visiting. 

Dynamic Creation of Specific Types at Runtime in ActionScript 3

I've seen this problem bandied about quite a bit; how do I create an instance of a given type directly from the Class, without having to declare an instance of the type somewhere to force the import?

Here's how I do it:

I take an incoming string specifying a key in an object I've created; these keys maps to Class types. I then process the class type based on the key; the trick is using getQualifiedClassName, which gets the absolute namespace, which avoids having to do the instance declaration. You could technically create that myTypes object by going through the app domain, but my need is generally satisfied by just making a table of the types available this way.

I then create the new instance and return it. It's typed as "*", but that's to be expected since I can be returning almost anything. You could always return something like { type : myType, instance : newInstance } or something if you wanted to be able to maintain the incoming typeName arg, and of course you can get that info after the fact by inspecting the returned object.

private var _myTypes : Object = { "MyType1" : MyType1, "MyType2" : MyType2 };
private function createInstance ( typeName ) : *
{
     var getType : String = flash.utils.getQualifiedClassName ( _myTypes [ typeName ] );
     var desiredClass : Class = flash.utils.getDefinitionByName ( typeName ) as Class;
     var newInstance : * = new desiredClass ( );

     return newInstance;
}

"JavaScript Patterns" Sandbox Pattern Correction

More from the JS world; I've been evaluating some ways to simplify the ponderous namespace typing in my javascript framework. It would appear, at this point, that the leading way to do this is by using the Sandbox pattern. As I understand it, the pattern is heavily used by the Yahoo YUI guys. It provides some other interesting capability as well, such as being able to create separate "app domains" as it were, that use different modules based on what you feed the Sandbox constructor. These different sandbox domains can be nested, etc. It's truly a plumbing pattern that seems to have a lot of potential to make your app more flexible and modular, as well as making the code easier to write/read.

 So far, I get it, and I believe it will be useful, but I'm still evaluating.

 I discovered that a reference implementation for the Sandbox pattern is in a book I already have; JavaScript Patterns. However, the reference pattern allows for feeding the desired modules for a given sandbox instance by a list of names, an array, or a '*' (meaning, use all available modules that have been registered with the Sandbox's static "modules" object literal).

 The problem is in the check that looks to see if you used "*".

if ( !modules || modules === '*' ) {
        modules = [ ];
        for ( i in Sandbox.modules ) {
            if ( Sandbox.modules.hasOwnProperty ( i ) ) {
                modules.push ( i );
            }
        }
    }

This causes an error, because the check will never evaluate to true; modules will never be as type "string", because earlier on it's typed as an array. So 'all available modules' will never get added to the Sandbox instance.

 The fix; add 'toString ( )' to the check (it makes sense, you're using absolute equality against '*', which is a string):

if ( !modules || modules.toString ( ) === '*' ) {
        modules = [ ];
        for ( i in Sandbox.modules ) {
            if ( Sandbox.modules.hasOwnProperty ( i ) ) {
                modules.push ( i );
            }
        }
    }

And the check will proceed as intended.

 As always, thanks for visiting.

"setXMLFromString" Extension for EditableGrid

EditableGrid is a very useful datagrid and chart component. It's written in JS, and after the usual short wrestling match/learning curve to incorporate into my projects, which these days are very data display heavy; lots of charts, tabular data that has to be interactive, I was pleased with the usage and display results.

You can find out more about EditableGrid here:

www.editablegrid.com

However, there is, from what I can see, one major API oversight, and forgive me if it's there but I looked in docs and source and couldn't find it: you can't set the xml from a string or dom object directly. You have to either fake a url call in the loadXML api call, or some other such hacky voodoo.

So I wrote a very simple extension for EditableGrid that allows you to do it straightforwardly.

Create a javascript file, call it, "useXMLString.js". Put it in the extensions directory of your EditableGrid structure.

Put this function in it.

EditableGrid.prototype.setXMLFromString = function ( theXMLString ) {

    if ( window.DOMParser )
    {
        var parser = new DOMParser (  );
        this.xmlDoc = parser.parseFromString ( theXMLString, "application/xml" );
    }
    else
    {
        this.xmlDoc = new ActiveXObject ( "Microsoft.XMLDOM" ); // IE
        this.xmlDoc.async = "false";
        this.xmlDoc.loadXML ( theXMLString );
    }

    this.processXML ( );
    this.tableLoaded ( );
}

Not much to see; I add a function to the EditableGrid prototype, use typical "browsers or IE" DOM parsing to turn the string into a DOM object, set the xmlDoc object of EditableGrid to that DOM object, then run the processXML and tableLoaded functions. This is pretty much how the usual EditableGrid.loadXML function works (you can see it in the EditableGrid source), I'm just taking out the need to use a URL.

Save it, and add it to your imports, the way you'd add any other extension to a JS component.

Now, don't call myEditableGrid.loadXML ( url ). Substitute myEditableGrid.setXMLFromString ( theXMLString ).

That's it, works like a charm. Note that you are responsible for making sure your XML string is a valid object.

As always, thanks for visiting.

JS: Why not just use [fill in name] framework?

I get this question when I discuss why I'm working on my own JS framework.

There's a lot of great frameworks out there nowadays; MooTools seems to be the emerging favorite. The PureMVC port uses it, all kinds of other component sets and whatnot seem to find it useful, so you end up installing it along with certain libraries, and so forth. Prototype, Backbone, YUI, and any number of other ones that are the thing du jour, are cropping up and so on.

I've been in development for about 20 years now. In that time, I've seen a LOT of frameworks, many of which no longer exist, and many that caused a lot of headaches in the long run, especially if community support or interest in them fizzled out.

To that end, I've decided, as a developer:

- Frameworks are a good thing as far as consistency, integration of developers into a codebase, and so forth. In fact, I have seen this benefit realized so absolutely, that there is no doubt in my mind that your typical project will benefit from at least some degree of plumbing framework.
- Frameworks can become a problematic thing if developers are allowed to work around, or abandon, the framework.
- Frameworks can be a horrible thing if you picked the wrong one, and/or your developers just stop using it because they think it's unnecessary.
- Frameworks should be unobtrusive; you should barely even know it's there if you're not using it, and it should add very little bulk to your codebase.

As a qualifier, I'm not talking about component frameworks. I'm talking about architectural frameworks.

So, I've come to believe, that along with a framework, comes the responsibility of educating your developers in it's use, and ensuring they use it. This means reviewing their code and making sure they're not cheating (e.g. instead of using a notification and/or event to exchange info, they are accessing "parent"...you see this ALL the time, "because it's easier/less/code/some-other-rookie-reason").

What's my point?

At this time, the notion of the "real" JavaScript developer, is still fairly immature. Crockford's book pretty much rules the roost as far as how to "think" in terms of JavaScript. This statement may not seem true to the Sencha engineers or Yahoo guys. But for your typical developer out there, particularly transitioning from the world of ActionScript, the path to writing solid, consistent code, and what tools to use, is unclear. Aside from JQuery for DOM manipulation, nothing is certain.

The biggest problem appears to be, the nature of JavaScript itself. It's about using functions as objects, and dynamically altering those objects to be useful in the context you need it for. Duck typing, shallow hierarchies, and such are what JavaScript is all about.

But...most developers aren't used to that, and many frameworks attempt to hide this way of thinking in favor of providing an artificial structure that "programmers" are more comfortable with, that being, classes, inheritance, interfaces, and so forth.

At this point in time, I think that's a mistake. We need to learn to think in JavaScript first, and find out how we can avoid mistakes we've made in the past with things like AS1 (excessive prototype manipulation, being dynamic to the point of being unable to tell one object from another without extensive duck typing and/or "flag programming", abuse of globals, etc.) while at the same time evolving the known patterns we're all familiar with to take advantage of strengths JavaScript offers, particularly, what exactly the power of "functions as first class", and the differences in how JavaScript resolves scope, means.

So to that end, I'm writing my own framework, with what I know about UI development and what I've observed over the years works (and doesn't), without losing site of the fundamental nature of ECMAScript development, which is different than classic OOP. Call it FOP (Function Oriented Programming), or what have you. But don't try to turn it into something it's not.

If you've concluded at this point that I'm some kind of framework hater, you're wrong. I have used PureMVC and RobotLegs extensively, have background in Cairngorm (which was never a good idea for UI dev imho) and experimented with many others. I like RobotLegs the best for one main reason; it is unobtrusive. It can live in your app and you can use it, or not use it, and easily combine it with other libraries. And it use EXTREMELY useful, and EXTREMELY powerful. This is in contrast to say, JSExt, which basically requires that you become a JSExt developer. Again, I have worked with JSExt a bit, and it's powerful and useful. If you're project is right for it, the benefit will be great. But it's hardly unobtrusive.

My indicators of success for my own framework will be:

- It should not require any other library unless that library has been proven stable for years and is in massive adoption by the community and the enterprise (e.g. JQuery, which my framework uses).
- It should be small. Very small. Hopefully, a handful of utility classes and possible a couple of prototype mods, but that's it.
- It should be powerful.
- It should foster decoupling and encapsulation of concerns.
- It should be repeatable and consistent.
- It should not hide the fundamental nature of javascript, which has nothing to do with classes as we know them.
- Other developers should find the paradigm familiar, e.g. more-or-less MVC.

So far, so good. I'm working on a very large project, currently in AS, and helping the company develop the strategy to go to JS/HTML (what many incorrectly call "an HTML5 app", which has the same meaning to me that the "Web 2.0" moniker had).

As always, thanks for visiting.