Thoughts on how to evaluate code

I met someone at Startup Camp2 who was non-techincal but had an idea that required technical expertise.

She faced the typical problem of judging and evaluate software code in order to make sure the people she hired were:

  • competent
  • making progress

This is of course hard especially in the beginnings of a project when so much is really building infrastructure code and configurations, none of which involves ‘visible’ progress.

I whipped off a quick email in response that even after sleep I still rather like:

  • Maintainability
  • Performance

These are your 2 key metrics. Poorly written code fails in these two areas.

Maintainability


Commented Code

Look through the code. Do all large methods have well-written comments that *you* can understand. You may not understand all the details but if there are no comments or the English is poor, this is bad. Any other developer coming later is going to have a hard time understanding what the original developer was trying to do and will probably create bugs when adding new features.

Key point: The comment should talk about *why* not just *what* is being done. The developer must describe all convoluted (to you) code in a *written* comment. This comment should be understandable to you, the layperson, to a reasonable degree. Chances are very good that, if he/she cannot that:

  • He is not quite certain himself what it does
  • He probably has not thought through completely all the issues around this code.
  • It has lots of bugs.

Be careful not to get lost in the weeds here. Have the developer take you through the high-level code, not the low-level stuff. Low-level stuff will distract you from seeing the bigger picture. You may want a friend developer who knows the language in question but be prepared to be able to fly solo on this after a few reviews.

Sample comment:
/**
 * Application State Object that tracks the current state of a flow. Holds any
 * state information related to a specific flow
 *
 * Each flow state has all the information to run the flow and re-enter it if
 * needed.
 *
 * defines an actively executing flow. Each FlowState has an attached
 * Flow which is the instantiated definition. This copy is made to avoid
 * problems with flow definitions changing while an instance of a flow is
 * active.
 */

Integration Tests/Unit Tests:

These are automated tests that anyone can run from the command line ( i.e. should not require bringing up a development environment.) You should be able to run a command line tool that reports number of tests run and the code coverage of those tests. These tests should include running something like selenium that will bring up a browser and run through your site.

Packaging

You should have a set of clear step by step instructions to get from a brand new machine to running the tests to bringing up the service. You need to be able to verify this yourself. Using only the directions only as written i.e. *no help from anyone* can you get the machine set up, source code downloaded from a source code repository**, compiled, and running? You should be able to type ‘http://localhost/’ and see your website.

[**Run away from any developer that doesn’t understand source code repositories. They are your insurance that 3 months into development the developer’s machine crashes and everything is gone.]

Can you bring up the development environment and start the product following the written directions by yourself? This avoids the possibility/probability that the developer’s machine is magically configured and only his machine is set up just so to build the product. Believe it or not, I have worked at large companies that are hair-pulling experiences because everything has to be magically configured to build the product.

Performance


How many people are going to hit your web server? What is the peak load going to be? What kind of response is the developer giving about issues like scalability?

Big issue here. Have the developer create jmeter tests that show how the server behaves under load. When running a jmeter test look at memory usage and CPU % on the server and that database. Ratchet up the number of jmeter users until the service just dies. Is that number acceptable? Look into making the service scalable using Amazon’s ec2 service. Ask questions about how much memory each logged in user takes. If each logged in user takes 1 megabyte of memory, you will only be able to have 300 or so users at a time/machine!

Any developer worth anything knows to use a database and well but they are not experts. Spend the money for a day or two of a database expert’s time. Have them take a look at the queries your service runs against the database. Have he/she do at least a little bit of tuning (this will be on-going process) but could easily allow the service to run 10x – 100x better.

This entry was posted in amplafi, code review, management, technical. Bookmark the permalink.

5 Responses to Thoughts on how to evaluate code

  1. Renat Zubairov says:

    I wouldn’t consider performance a critical acceptance criteria because you could get to the much painfull resuilts such as _PREMATURE OPTIMIZATION_ which is a bigger evil that badly designed SQL statements, or Vectors inside single-threaded environment.
    What I would consider most important is unit code coverage, if you don’t have that you can’t evolve, you can’t put new developers on the source code without regressions. You wouldn’t be possible to optimize the performance when it will be needed because of lack of unit tests.
    Also if we consider performance issues we should think about the fact, that actually big number of users is _very_hard to get in the first place 🙂

  2. patrick says:

    Hi Renat,

    You are correct that not every website needs to be able to handle the load that amazon handles. And it is important for there to be reasonable expectations as to the needs. So when doing performance testing the simulated load needs to be inline with expected traffic.

    However, consider these issues:

    1. The website may be highly successful, or be slashdotted/digged/etc. Being able to handle the ‘success’ of a favorable review is critical. Otherwise, a positive review (and uptick in traffic) is lost.
    2. The person reviewing/specing the product is non-technical. This means they need fairly simple metrics by which to measure the capabilities of the delivered code.
    3. Throughput, load numbers, etc are easily quantified in legal terms. Thus a simple, unequivocal, enforceable contract can be written between developer and employer
    4. Lastly, it is very hard to produce lousy code that still performs well.

    The issue of premature optimization is an internal concern for a developer, but it is never a concern that you will hear from a business owner. They just want to know if the code will met their needs.

  3. Actually I completely disagree with the comments bit. Good clean code is self-documenting. Comments are a necessary evil, and are not something you should strive towards. They are a fallback when there is no elegant and self-documenting method to go about implementing something. I know that the best code I’ve seen actually -only- comments the hacks or unexpected bits which are very few and far between.

    Ideally it is in the function names and the variable names that you will see most of the ‘commenting’ and remember that while comments can lie about what code does, but code cannot lie about what it does. Comments can become outdated very easily and then become not only unhelpful but actually detrimental to development. They also impose more overhead in maintenance which is easily ignorable because comments don’t actually execute and this is primarily how they get out of date.

    Let me suggest reading the book “Clean Code” by Robert C. Martin which covers many ideas in very good detail. http://www.amazon.ca/Clean-Code-Handbook-Software-Craftsmanship/dp/0132350882

  4. Pingback: Just wondering…. » Blog Archive » Not commenting code is dangerous to your career

  5. patrick says:

    Hi Mike —

    Thanks for taking the time to stop by and comment. Its possible that you have seen better code than I have (or that I can write). I do agree; clean solid code makes “what” the code is doing clear.

    However there are some key thoughts that you should consider.

    http://www.sworddance.com/blog/2009/03/04/not-commenting-code-is-dangerous-to-your-career/

    and http://www.sworddance.com/blog/2009/03/04/code-review-7-comment-the-why-not-the-what/

    Lastly, this post was directed at non-technical types who are paying the bill. If a developer can not clearly explain and show progress — why should they continue to pay any money?

    So I am curious how you would solve these issues without comments?

    Thanks again for stopping by.

Leave a Reply

Your email address will not be published. Required fields are marked *