Play game here

October 30, 2007

Just added 2 free games from DynamicDrive. I love this site for its great resources. Besides, I added fading effect to pop up the game div at center. You can now play these games from this page, just click any of the game link from the top left menu !!

Would you please play once and put your scores on comment ? :)

Thanks for your encouragement.


Macadamian’s Code Review Checklist

October 25, 2007

At Macadamian, we practice what we preach with peer code reviews. Before we commit any code to source control, we check it for compliance with this list.

We’ve made the checklist public for the use of software development teams implementing code review as part of their process. For more information about code review processes and software development best practices, read check out the Critical Path newsletter – it’s free, too!

General Code Smoke Test
Comments and Coding Conventions
Error Handling
Resource Leaks
Control Structures
Performance
Functions
Bug Fixes
Math


General Code Smoke Test

Does the code build correctly?
No errors should occur when building the source code. No warnings should be introduced by changes made to the code.

Does the code execute as expected?
When executed, the code does what it is supposed to.

Do you understand the code you are reviewing?
As a reviewer, you should understand the code. If you don’t, the review may not be complete, or the code may not be well commented.

Has the developer tested the code?
Insure the developer has unit tested the code before sending it for review. All the limit cases should have been tested.

Comments and Coding Conventions

Does the code respect the project coding conventions?
Check that the coding conventions have been followed. Variable naming, indentation, and bracket style should be used.

Does the source file start with an appropriate header and copyright information?
Each source file should start with an appropriate header and copyright information. All source files should have a comment block describing the functionality provided by the file.

Are variable declarations properly commented?
Comments are required for aspects of variables that the name doesn’t describe. Each global variable should indicate its purpose and why it needs to be global.

Are units of numeric data clearly stated?
Comment the units of numeric data. For example, if a number represents length, indicate if it is in feet or meters.

Are all functions, methods and classes documented?
Describe each routine, method, and class in one or two sentences at the top of its definition. If you can’t describe it in a short sentence or two, you may need to reassess its purpose. It might be a sign that the design needs to be improved.

Are function parameters used for input or output clearly identified as such?
Make it clear which parameters are used for input and output.

Are complex algorithms and code optimizations adequately commented?
Complex areas, algorithms, and code optimizations should be sufficiently commented, so other developers can understand the code and walk through it.

Does code that has been commented out have an explanation?
There should be an explanation for any code that is commented out. “Dead Code” should be removed. If it is a temporary hack, it should be identified as such.

Are comments used to identify missing functionality or unresolved issues in the code?
A comment is required for all code not completely implemented. The comment should describe what’s left to do or is missing. You should also use a distinctive marker that you can search for later (For example: “TODO:francis”).

Error Handling

Are assertions used everywhere data is expected to have a valid value or range?
Assertions make it easier to identify potential problems. For example, test if pointers or references are valid.

Are errors properly handled each time a function returns?
An error should be detected and handled if it affects the execution of the rest of a routine. For example, if a resource allocation fails, this affects the rest of the routine if it uses that resource. This should be detected and proper action taken. In some cases, the “proper action” may simply be to log the error.

Are resources and memory released in all error paths?
Make sure all resources and memory allocated are released in the error paths.

Are all thrown exceptions handled properly?
If the source code uses a routine that throws an exception, there should be a function in the call stack that catches it and handles it properly.

Is the function caller notified when an error is detected?
Consider notifying your caller when an error is detected. If the error might affect your caller, the caller should be notified. For example, the “Open” methods of a file class should return error conditions. Even if the class stays in a valid state and other calls to the class will be handled properly, the caller might be interested in doing some error handling of its own.

Has error handling code been tested?
Don’t forget that error handling code that can be defective. It is important to write test cases that exercise it.

Resource Leaks

Is allocated memory (non-garbage collected) freed?
All allocated memory needs to be freed when no longer needed. Make sure memory is released in all code paths, especially in error code paths.

Are all objects (Database connections, Sockets, Files, etc.) freed even when an error occurs?
File, Sockets, Database connections, etc. (basically all objects where a creation and a deletion method exist) should be freed even when an error occurs. For example, whenever you use “new” in C++, there should be a delete somewhere that disposes of the object. Resources that are opened must be closed. For example, when opening a file in most development environments, you need to call a method to close the file when you’re done.

Is the same object released more than once?
Make sure there’s no code path where the same object is released more than once. Check error code paths.

Does the code accurately keep track of reference counting?
Frequently a reference counter is used to keep the reference count on objects (For example, COM objects). The object uses the reference counter to determine when to destroy itself. In most cases, the developer uses methods to increment or decrement the reference count. Make sure the reference count reflects the number of times an object is referred.

Thread Safeness

Are all global variables thread-safe?
If global variables can be accessed by more than one thread, code altering the global variable should be enclosed using a synchronization mechanism such as a mutex. Code accessing the variable should be enclosed with the same mechanism.

Are objects accessed by multiple threads thread-safe?
If some objects can be accessed by more than one thread, make sure member variables are protected by synchronization mechanisms.

Are locks released in the same order they are obtained?
It is important to release the locks in the same order they were acquired to avoid deadlock situations. Check error code paths.

Is there any possible deadlock or lock contention?
Make sure there’s no possibility for acquiring a set of locks (mutex, semaphores, etc.) in different orders. For example, if Thread A acquires Lock #1 and then Lock #2, then Thread B shouldn’t acquire Lock #2 and then Lock #1.

Control Structures

Are loop ending conditions accurate?
Check all loops to make sure they iterate the right number of times. Check the condition that ends the loop; insure it will end out doing the expected number of iterations.

Is the code free of unintended infinite loops?
Check for code paths that can cause infinite loops. Make sure end loop conditions will be met unless otherwise documented.

Performance

Do recursive functions run within a reasonable amount of stack space?
Recursive functions should run with a reasonable amount of stack space. Generally, it is better to code iterative functions.

Are whole objects duplicated when only references are needed?
This happens when objects are passed by value when only references are required. This also applies to algorithms that copy a lot of memory. Consider using algorithm that minimizes the number of object duplications, reducing the data that needs to be transferred in memory.

Does the code have an impact on size, speed, or memory use?
Can it be optimized? For instance, if you use data structures with a large number of occurrences, you might want to reduce the size of the structure.

Are you using blocking system calls when performance is involved?
Consider using a different thread for code making a function call that blocks.

Is the code doing busy waits instead of using synchronization mechanisms or timer events?
Doing busy waits takes up CPU time. It is a better practice to use synchronization mechanisms.

Was this optimization really needed?
Optimizations often make code harder to read and more likely to contain bugs. Such optimizations should be avoided unless a need has been identified. Has the code been profiled?

Functions

Are function parameters explicitly verified in the code?
This check is encouraged for functions where you don’t control the whole range of values that are sent to the function. This isn’t the case for helper functions, for instance. Each function should check its parameter for minimum and maximum possible values. Each pointer or reference should be checked to see if it is null. An error or an exception should occur if a parameter is invalid.

Are arrays explicitly checked for out-of-bound indexes?
Make sure an error message is displayed if an index is out-of-bound.

Are functions returning references to objects declared on the stack?
Don’t return references to objects declared on the stack, return references to objects created on the heap.

Are variables initialized before they are used?
Make sure there are no code paths where variables are used prior to being initialized. If an object is used by more than one thread, make sure the object is not in use by another thread when you destroy it. If an object is created by doing a function call, make sure the object was created before using it.

Does the code re-write functionality that could be achieved by using an existing API?
Don’t reinvent the wheel. New code should use existing functionality as much as possible. Don’t rewrite source code that already exists in the project. Code that is replicated in more than one function should be put in a helper function for easier maintenance.

Bug Fixes

Does a fix made to a function change the behavior of caller functions?
Sometimes code expects a function to behave incorrectly. Fixing the function can, in some cases, break the caller. If this happens, either fix the code that depends on the function, or add a comment explaining why the code can’t be changed.

Does the bug fix correct all the occurrences of the bug?
If the code you’re reviewing is fixing a bug, make sure it fixes all the occurrences of the bug.

Math

Is the code doing signed/unsigned conversions?
Check all signed to unsigned conversions: Can sign completion cause problems? Check all unsigned to signed conversions: Can overflow occur? Test with Minimum and Maximum possible values.


Interesting Code Review "Facts"

October 25, 2007

The benefit of code reviews is undeniable.

Yet, code reviews don’t happen in most software development organizations. And for good reason. The way code reviews are typically handled uses an enormous amount of people and time resources. Even with system quality improvements developers may not feel the effort is worth the cost.

It turns out you don’t need a high overhead code review process to get results. At one company I worked at I created a very low overhead automated code review system that enabled every line of code to be reviewed before it was checked-in.

Here are some ideas of how code reviews can be made practical in your organization.

  1. Interesting Code Review “Facts”
  2. Code Review Standard
  3. Perspective Based Reviews
  4. Deciding If Code Reviews Work
  5. Are Code Reviews Necessary With Pair Programming?
  6. Code Review Resources



Interesting Code Review “Facts”

My initial impression of code reviews is that they were heavy and bloated and a waste of time. I was wrong on all counts. Here are some interesting facts I found in the research:

  1. Using two inspectors is as effective as using four or more.
  2. Using an asyncronous process is as effective as a meeting based process for code reviews. For design reviews a meeting based approach may be more effective.
  3. One review session is as effective as multiple inspection sessions.
  4. Scenario or perspective based reviews were more effective than add-hoc and check-list based reviews.
  5. Inspections are effective.
  6. Inspecting upstream process like requirements and designs is very effective.

The implication is that the complexity and overhead of Fagan like code inspections are not needed and a semi-automated method will yield improved system quality. Requirements and HLDs seem to benefit from a more structured inspection process, which makes a certain amount of sense.



Code Review Standard

This is the list of some policies I would use to create a code review standard for my organization. Take what’s here, read the review on code research, and figure out what will work best for your organization.

The key point to keep in mind is that the heavy weight nature of code reviews is not true. You can create a code review system for your group that is light enough that you can review every change in your entire system.

Every Line of Code Must Be Reviewed Before it is Checked-In

Do not allow code to be checked-in unless it has been reviewed, fixed, and rereviewed. If you can’t do this then your review process isn’t fast or light enough. It can be done.

Reviewing code after it has checked-in is next to useless as everyone is exposed to the unreviewed code.

Code Must Be Integrated, Compiled, Unit Tested, and System Tested Before Review

Spending time on code that is just going to change or doesn’t work is a massive waste of time. The entrance requirements for a code review are:

  • Code should compile without errors or warnings (according to coding standard).
  • Code should be already be integrated with its parent branch.
  • Code should be fully unit tested.
  • Code should already pass the system tests where possible.

Develop Perspective Based Reviews

For the issues that are important in your software consider developing a Perspective Review for each issue. Perhaps special hotspots for you are internationalization, memory corruption, memory usage, threads, semaphores, etc. An example can be found in Semaphore Perspective Code Review.

You can then use the perspectives to assign roles to review team members.

Use Meetingless Asyncronous Reviews

You don’t need to hold a big meeting for a review. Everyone can review the code when they can find time. Simpy have a done-by date when the review must be completed.

Everyone on the review must review the code. If a person can’t perform the review then the team needs to elect someone else to perform a similar role in the review.

Meetingless aysncronous reviews are fast and light, yet they are still effective. With the right tool support you can easily review every line of code in your system.

Use Between 2 and 5 Relevant Reviewers

Too many reviewers wastes everyones time. Keep the number of reviewers small and relevant to the code being reviewed.

Assign Reviewers Roles and Perspectives

It’s almost impossible to review a lot of different issues in a lot of code. People get tired and they stop noticings things.

The way around the getting tired issue is to use perspective reviews. Create a perspective for each important issue category your are concerned about. Assign the perspectives to people on the review team. Because they are only reviewing for issues in their perspective they will do a better job because they can stay more concentrated. This doesn’t mean they can’t find other issues as well, but they responsible for their perspective.

For example, if using semaphores correctly is important in your software and the code has semaphores, then assign someone the role of reviewing semaphore usage. An example can be found in Semaphore Perspective Code Review.

All Review Communication Should Go to the Review List and be Logged

Part of the benefit of a review is that people learn about the system being reviewed. This learning feature is facilitated by broadcasting email communication between the review team and saving all communication so it can be read by other people later.

Do Not Redesign in the Review

Make a note and schedule design issues for for a later time.

Developers always think they can do stuff better. Take these issues off line unless the issue is that requirements are not being met. Requirements not being met is not the same as you could have done it better.

Do Not Cover Coding Standards Violations in the Review.

Send violations via email or in person.

Talking about violations only gets everyone angry and is a waste of time.

Code Is Rereviewed Until it Passes

Code isn’t reviewed once and then forgetten. Any changes made have to be rereviewed. If you think this is too slow then your process isn’t light enough.

No reviewing all changes makes the process useless as people will just ignore suggestions or introduce new bugs in any changes.

All Issues Must be Fixed, Marked as Not an Issue, or Marked as Bug

Any issue brought up to a developer must be handled. A developer just can’t ignore issues because they think it’s stupid. Every issue must:

  • Fixed.
  • Marked as Not an Issue. If the developer and the reviewer can’t decide between themselves if an issue should be fixed or not, then the review team gets to decide. If there is only one reviewer then bring a manager in or another team memember.

Automate Your Code Review System

You can make your process light enough by building it into your build system. If your process isn’t light enough work on until it is.

Review All Code on Private Branch Before a Merge

Code developed on a private branch doesn’t need to reviewed during development. But before the code is merged into a parent branch all code changes must go through the complete review process. For this reason, development of a large scale feature, may still want to perform reviews on the private branch because that can speed up the merge process.

Of course, try not to have branches separate from the mainline, but for large features that take a long time to develop you will often need separate branches.

Review the Right Scope of Changes

You don’t have to review every line of code in every module that has changed. Certainly if a module is new it must be completely reviewed. Other than that you may be able to just review the changed code. Though just reviewing changed code isn’t always possible. If you are performing a semaphore perspective review, for example, then you will need to go look at the code within in the scope of the sempahore as well.

Stick to Reviewable Issues

Develop your list of what issues can be reviewed and how they are to be reviewed. Usually this is in the form of check lists and perspective reviews. Don’t allow reviews on other items without changing what can be reviewed. Otherwise people spend endless time on off-topic arguments.

Review Team Reponsible for Deciding Issues

If there is a conflict on any part of the review then the review team is responsible for handling it. That’s the only way the review process will be light enough to work.

Keep it Cool

Nobody is perfect. The attitude of the review should never be personal, it should always be professional, with the goal of improving the system and the people building the system. Keep your tempers.

Don’t blame people for bugs. Work together to make things better. No finger pointing! Not ever!

Meetingless reviews can help keep the anger down, but it can make it worse too. When people are in the same room anger can ramp up really quick. And we know in email it’s very easy to say something that can be take wrong. Raise the awareness of these issues in your team.

A good rule is to Never Assume An Attack. If you find yourself getting angry, assume it’s a misunderstanding, not an attack.

No Managers

Unless a manager has something to add to the reviews then they shouldn’t be involved. Issues should be decided by the review team. Managers always have to run to a different meeting, they don’t have time slots open for meetings, and they generally don’t add technical input. So you don’t need managers as part of the review process.

If a Bug Was Not Caught in a Review Figure Out Why

If a bug happens after a review then track down why each bug wasn’t found and then change your development process somehow to try and prevent that bug in the future.

This is not always possible as running full tests are often impossible, but it should be mostly possible.

I would create a bug for each bug to track down why it wasn’t caught. Because this issue is more serious than just the review. It means the unit test, the system test, and the review did not work.

Review Upstream Documents Too

Reviewing product requirement documents, specs, standards, etc can provide excellent return on value. Make sure those products are reviewed as well.

Feed Leasons Learned Into the Team and Documentation

If issues come up during the review that everyone in the team would benefit from, then have a way to make wisdom public.

I would recommend a development wiki where you can write documentation on anything useful that people come up with.

Plug Reviews into Your Source Code Control System

I have done this through change check-in comments. Each change has to be reviewed before it is checked-in. The submit comment must contain a review ID that points to some document containing the review status for the change that is about to be submitted. The code is prevented from being submitted without a valid passed review. If you are able to automate your code review system all this works quite quickly and painlessly.



Perspective Based Reviews

Before my reseach into code reviews I had never before heard about perspective based reviews . Perspective based reviews are done from the point-of-view of various project stakeholders. This is as apposed to everyone doing an add hoc or checklist based review. A perspective would be a user, requirements person, hardware interface, exception handling, mutex usage, memory usage, etc. The thought is they are more effective because reviewers are concentrating on a particular issue instead of trying to review everything. Do one thing well i guess.

Reviews are driven by scenarios. A scenario tells the insepector how to go about reading the documentation from particular perspective and what to look for. Clearly there’s a lot to the scenarios.

Perhaps people could be assigned in various review roles. Or maybe some people can perform reviews only from a certain perspective.



Deciding If Code Reviews Work

How do you know if code reviews are working for you? Look for

  • Increased in test execution rate. The tests suites runs through its test suite. Unstable software can’t do this.
  • Increased pass rate.
  • Reduction of critical bugs in QA.
  • Reduction of field issues.
  • Subjective improvements in quality.

Try to measure the issues to verify if code reviews are working for you.



Are Code Reviews Necessary With Pair Programming?

An issue that comes up is the relationship between code reviews and pair programming . Do you still need code reviews if you pair program?

If you don’t pair program then you definitely should perform code reviews. That one is easy. Unit tests and system tests simply aren’t good enough to replace code reviews.

After that it depends.

It depends on the product you are developing, it’s importance, it’s complexity, and how critical it is that there be no errors.

Pair programming is an excellent form of active code review, but there are many products where I don’t think two eyes are enough. Some systems are complex enough, some domains are complex enough, and some conseuqneces are severe enough that I want more eyeballs on them before the code is let loose in a real code running system.

When pair programming is used you could decide what code should be reviewed by subsystem or by the type of change involved.

For example, code in a key algorithm may need to be reviewed by the impacted parties. Perhaps a drive change needs to be reviewed. Perhaps when a piece of code is written new or completely rewritten, it should be reviewed. Perhaps if semaphores are added to code that would trigger a review because it changes the system a great deal.

Your mileage may vary of course.


Charecteristics of commercial software

October 25, 2007

Commercial Softwares – A term with whom most of the software professional, including me, are familiar with. There are thousands of different commercial software in different areas; even though all of them share some common properties such as: built by many, expandable, maintainable, understandable, and stable.

Lets see what are those:

  • Built by many: Software that is built by a team inherently means that no single person has firsthand knowledge of all of the code. It also means that good communication among the team members is critically important to the software’s success. Communication has to start at the beginning; everyone on the team must share an understanding of what exactly is being built, for whom, and when it is needed. By understanding the customers and their requirements, you can always go back and ask yourself, “Is this feature I’m adding really necessary?” Understanding both the business and product requirements can help you avoid the dreaded feature creep or increasing scope that often delays products. Communication has to continue throughout the process: conveying interface designs so that integration among components runs smoothly; accurately and completely documenting your code; and conveying new member functions and their purpose to the Software Quality Assurance (SQA) and Documentation groups.

  • Expandable: You must be able to add new features quickly and extend existing features in commercial software. These requests often seem to come when an influential customer finds that they need just one more thing to solve their problem. Even worse is when a limitation is discovered only after the product is released. Whether or not code can be easily expanded to accommodate these requests is often a function of the design. While it may be as easy as adding a new member function, significant new features require that the design be extended. Put simply, you have to think ahead when designing and writing code. Just implementing to a specification, with no thought to future expansion or maintainability, makes future changes much more difficult. A further complication is that most software won’t allow features to be deprecated once the product is in use. You can’t simply remove a class that was ill conceived, as it is certain that if you do so, you’ll find a whole host of customers using it. Backward compatibility is often a key design constraint.

  • Maintainable: No matter how many unit tests are written, automated tests are run, or functional tests are executed, it is inevitable that bugs will be found after the product has been released. For commercial software, the goal of all of the rigorous testing and preparation is to ensure that any post-release bugs have minor consequences to the product. What becomes crucial is that these bugs be easily and quickly corrected for the next release. It is also likely that the person who implemented a particular component won’t be the person responsible for fixing the bug, either because enough time has passed that he has been assigned to a different project, or there are designated engineers responsible for support and maintenance. Given these constraints, maintainability must be designed into the software. There has to be an emphasis on solid design practices and clarity in coding styles, so that the software can be efficiently maintained without further complicating it.

  • Understandable: Commercial software often includes visible software interfaces. If you are building an embedded system library, it needs to be understandable to the application engineers within your company. If you are building a software library, the interface you present must be easily understood by your customers so that they can use it to build their own applications. This doesn’t just mean that naming conventions must make sense, but more importantly that your design and your use of language elements, like templates, are clear and appropriate.

  • Stable: Commercial software must be stable; that is, it must be able to run for extended periods of time without crashing, leaking memory, or having unexplained anomalies.


Me, in Cherapunji

October 23, 2007


Me, in Cherapunji
Originally uploaded by Happy-Man

This is a observation point at Cherapunji, India. We had a great expectation about the seven-sister fall that we read on internet before visiting India. The local guide told us in rainy season the fall becomes large enough and looks great.

Hard luck !! We went in winter :(