You are currently browsing the Agile archives.

Code Review Essentials

Friday, June 3rd, 2022

Code Reviews are an essential part of Software Engineering, providing numerous benefits for teams and the products they deliver. Having spent a significant amount of time conducting them for many years now, in this article, we will touch upon some key aspects to consider which, generally speaking, are of particular importance.

Similar to functional testing, Code Reviews provide a unique set of quality controls which help ensure standards are upheld; affording teams the ability to verify a number of critical concerns early on and within the confines of engineering specific constructs. This almost certainly yields a higher return as the time investment required to address issues at this stage requires minimal involvement across teams and functions.

Code Reviews also serve to aid in the verification and upholding of best practices, standards, and conventions across teams and within organizations. These standards can cover a broad range of concerns such as consistency, facilitation of reuse, scalability, security, optimization, readability, simplification, and any other auxiliary criteria specific to a given organization.

Additionally, the Code Review helps to confirm that requirements have been fulfilled in the context of the underlying feature being reviewed as, it is not uncommon for developers to misinterpret requirements.

Likewise, developers are generally focused on solving various small problems in a very particular and limited scope. Because of this, it is inevitable that opportunities will be missed, and oversights will be made. One of the primary responsibility of the Reviewer is to provide a holistic and broad perspective which takes into account not only the soundness of the code being reviewed, but also how it measures, complies, and integrates in the context of the larger system as a whole.

By having another set of eyes, so to speak, we arm ourselves with a very important second line of defense, as well as an agent for opportunity.

One of the most beneficial aspects of Code Reviews is the investment in overall knowledge throughout the team; and ultimately, the ROI it provides. As such, core to the Code Review is the proliferation of knowledge. This applies to both the Reviewee, and the Reviewer alike.

For the Reviewee, when areas of improvement, best practices, optimizations, abstractions and the like are outlined, an opportunity is presented for one to learn new (often improved) techniques which they may not have been aware of otherwise. This holds particularly true for more junior developers who simply have yet to acquire the experiential knowledge obtained by their more senior counterparts. By learning from the experiences of others, the Reviewee can expedite their own growth as a Developer. Here, the expectation is that, overtime, each Reviewee will have fewer and fewer of the same review comments to address as they now have a dedicated platform (even if unofficially so) from which to continually learn.

For the Reviewer, Code Reviews provide an opportunity to share knowledge and insight, while affording one the ability to obtain a broader understanding of the system in its entirety, as this knowledge is vital to providing a successful review.

Additionally, it may be necessary for a Reviewer to devise and provide solutions to problems which the may not have encountered previously and, in order to be effective, a Reviewer must be confident in the feedback and solutions they are providing. This alone affords the Reviewer themselves the ability to gain a deeper understanding of their own knowledge, while also challenging themselves in order to obtain the information necessary to do so. Thus, for the Reviewer, Code Reviews present a tremendous opportunity to not only provide value to others, but also to obtain and enhance their own value as well.

In general, developers more or less tend to work in a rather silo’d manner, primarily focusing on one particular problem space (particularly in the scope of a given feature), and only collaborating when necessitated by DSMs, meetings, or when they or another team member runs into a problem and needs assistance. While much of this is a rather natural by-product of feature development, so to, can it be said that Code Reviews naturally cultivate collaboration; thus, collaboration can be built into our processes by default.

With Code Reviews, no one Developer is ever working completely on their own. This has numerous benefits, many of which have already been outlined above, yet perhaps one of the most significant benefits is that developers are much more likely to double check their work and submit something that they can be proud of when they know someone else on their team will be reviewing their work. Likewise, Reviewers, no matter how experienced, are much more likely to validate and double check their feedback for the exact same reasons. This alone lends itself to higher quality output across the board.

Key Aspects to Consider

While numerous aspects must be considered with respect to conducting Code Reviews, generally speaking, there are common considerations which by and large tend to hold true. While certainly not an exhaustive treatise, what follows is a brief outline of those I have found to provide particular value.

Atomicity: PRs should be atomic (relatively small in nature). If PR is excessively large, it should be rejected and the engineer should be informed to break out the PR into smaller submissions (generally these smaller submissions can be merged to an intermediary branch before being merged to the intended target branch). This is crucial as the surface area for mistakes and missed opportunities is proportional to the amount of code being reviewed. In addition, requiring PRs which are smaller in scope encourages developers to think in terms of smaller units of function and subsystems, which in turn leads to clearer separation of concerns, and encapsulation. As such, it is often helpful to impose a change threshold for submitted PRs.

Compatibility: Changes should remain backwards compatible and not introduce breaking changes (unless expressly coordinated across teams). Reviewers need not checkout each PR and explicitly test each feature being submitted, rather, they should always be cautious of breaking changes, particularly in terms of APIs (e.g. argument positions changing, etc.).

Consistency: PRs must fully adhere to well documented and established standards and conventions; typically supported via commit convention tooling. This is crucial as, consistency and conformity of standards leads to a unified codebase where developers can easily work across packages and features with very limited effort as, the overall structure and coding style is consistent; making it much easier to know where everything should, and is, defined, how modules are organized, and readability is immediate as formatting and structure remains the same across packages and modules.

Clarity: All modules, functions, classes, types, etc. are always be clearly named, defined, remain properly encapsulated, and reside within a logical and appropriate location.

Readability: Readability should be favored over excessive succinctness or overly “clever” implementations which do not read well. Conversely, overly verbose implementations are to be avoided as well. It is important to remain cognizant of the fact that code is read many, many more times than it is written. Moreover, when implementations become hard to reason about, that is often a sign of a poor implementation (usually the result of a specific unit doing too far much). Succinct, yet meaningful names must always be used. Strive to ensure code is self documenting in terms of its intention.

Reusability: Implementations must take reuse into account at all times; be it abstractions to common packages, abstractions within a particular project, or abstractions within a particular scope of a project. In addition, Reviewers should always be on the look out for additions which are redundant and should be removed and replaced with existing APIs available. This includes both internal APIs, as well as third-party libraries. Always ensure native APIs are being leveraged (Array.forEach, etc. rather than explicit for loops) as well as standard third party libraries (lodash.debounce, etc. rather than custom implementations). No redundancies should be introduced, and implementations should fully utilize existing APIs, Modules, Components, etc. throughout the available packages.

Simplicity: Solutions should always be implemented in the simplest way possible. Less is more, this extends down to each line of code. Keep things as simple as possible, but no simpler.

Scalability: Implementations must be performant and optimized to an acceptable and expected level – generalized optimizations must be made, and premature optimizations should only be suggested when necessary.

Securability: Implementations must be secure, keeping standardized security measures in place and ensuring attack vectors and cumulative surfaces are fully understood, accounted for, and securely addressed.

Discoverability: Documentation and / or related tools should follow specific conventions and remain succinct and to the point. Ideal documentation should provide a meaningful, yet brief description, followed by a useful example which speaks for itself (often, unit test expectations can be used verbatim here). On a related note, sources should not contain overly verbose inline comments as well. When, for example, a function has more lines of inline comments than actual implementation code, that’s usually a sign that the code does not read well, or the developer has merely been leaving “note to self” comments. In such cases, strive to provide ways to simplify the implementation such that it achieves better readability by being self documenting.

Accountability: It is crucial that all Team members are aware of the criteria against which their code will be reviewed as, doing so essentially holds developers accountable for ensuring they not only understand what is expected, but are diligent in reviewing their own work prior to submission. Developers should be encouraged to pre-submit PRs for performing a “self review” prior to officially submitting and / or assigning a reviewer. This approach is quite valuable as it provides the developer with a high-level overview of their changes outside of the environment they have been working in, and within the context of the branch to which their changes will be integrated.

While there are certainly other factors to consider when conducting Code Reviews, the above considerations touch upon some of the more fundamental aspects, with the key points hopefully being apparent as, perhaps the most important trait of a successful reviewer is in one’s ability to clearly express intent while also passing this knowledge on to others.

Test First Workflow – A Short Story

Saturday, February 2nd, 2013

As a depiction of the typical approach taken when solving a problem with Test First practices in mind, below is a brief excerpt from a recent conversation with a collegue who inquired of me as to how one generally goes about solving a problem using Test First methodologies. My explanation was rather simple, and read somewhat like a short story, though I describe it as being more of a step by step process from a Pair Programming perspective.

The general workflow conveyed in my description, while brief, covers the essentials:

  1. We have a problem to solve.
  2. We discuss the problem, asking questions as needed; then dig a bit deeper to ensure we understand what it is we are really trying to solve; and, most importantly, why.
  3. We consider potential solutions, identifying those most relevant, evaluating each against the problem; then agree upon one which best meets our needs.
  4. We define a placeholder test/spec where our solution will be exercised. It does nothing yet.
  5. We implement the solution in the simplest manner possible, directly within the test itself; the code is quite ugly, and that is perfectly fine, for now. We run our test, it fails
  6. We adjust our implementation, continuing to focus solely on solving the problem; all the while making sure not to become too distracted with implementation details at this point.
  7. We run our test again, it passes. We’re happy, we’ve solved the problem.
  8. We move our solution out of the test/spec to the actual method which is to be implemented, which, until now, had yet to exist.
  9. We update our test assertions/expectations against the actual (SUT). We run our test, it passes.
  10. We’re happy, we have a working, tested solution; however, the implementation is substandard; this has been nagging at us all along, so we shift focus to our design; refactoring our code to a more elegant, performant solution; one which we can be proud of.
  11. We run our test again, it fails. That’s fine, perhaps even preferable, as it verifies our test is doing exactly what is expected of it; thus, we can continue to refactor in confidence.
  12. We adjust our code, continuing to make design decisions and implementation changes as needed. We run our test again, it passes.
  13. We refactor some more, continuing to focus freely, and without worry on the soundness of our design and our implementation. We run our test again, it passes.

Rinse and Repeat…

While the above steps are representative of a typical development work-flow based on Test First processes, it is worth noting that as one becomes more acclimated with such processes, certain steps often become unnecessary. For example, I generally omit Step #5 insofar as implementing the solution within the test/spec itself is concerned; but rather, once I understand the problem to be solved, I then determine an appropriate name for the method which is to be tested, and implement the solution within the SUT itself, as opposed to the test/spec; effectively eliminating the need for Step #8. As such, the steps can be reduced down to only those which experience proves most appropriate.

Concluding Thoughts

Having become such an integral part of my everyday workflow for many years now, I find it rather challenging to approach solving a problem without using Test First methodologies. In fact, attempting to solve a problem of even moderate complexity without approaching it from a testing perspective feels quite awkward.

The simple fact is, without following general Test First practices, we are just writing implementation code, and if we are just writing implementation code, then, in turn, we are likely not thinking through a problem in it’s entirety. Consequently, it follows then that we are also not thinking through our solutions in their entirety, and hence our designs. Because of this, solutions feel uncertain, and ultimately leave us feeling much less confident in the code we deliver.

Conversely, when following sound testing practices we afford our team and ourselves an unrivaled sense of confidence in terms of the specific problems we are solving, why we are solving them, and how we go about solving them; from that, we achieve a concerted understanding of the problem domain, as well as a much clearer, holistic understanding of our designs.

Practices of an Agile Developer

Thursday, February 10th, 2011

Of the many software engineering books I have read over the years, Practices of an Agile Developer in particular continues to be one book I find myself turning to time and time again for inspiration.

Written by two of my favorite technical authors, Andy Hunt and Venkat Subramaniam, and published as part of the Pragmatic Bookshelf, Practices of an Agile Developer provides invaluable, practical and highly inspirational solutions to the most common challenges we as software engineers face project after project.

What makes Practices of an Agile Developer something truly special is the simplicity and easy to digest format in which it is written; readers can jump in at any chapter, or practically any page for that matter, and easily learn something new and useful in a matter of minutes.

While covering many of the most common subjects on software development, as well as many particularly unique subjects, it is the manner in which the subjects are presented that makes the book itself quite unique. The chapters are formatted such that each provides an “Angel vs. Devil on your shoulders” perspective of each topic. This is quite useful as one can briefly reference any topic to take away something useful by simply reading the chapters title and the “Angel vs. Devil” advice, and from that come to a quick understanding of the solution. Moreover, each chapter also provides tips on “How it Feels” when following one of the prescribed approaches. The “How it feels” approach is very powerful in that it instantly draws readers in for more detailed explanations. Complimentary to this is the “Keeping your balance” suggestions which provide useful insights to many of the challenges one might face when trying to apply the learnings of a particular subject. “Keeping your Balance” tips answer questions which would otherwise be left to the reader to figure out.

I first read Practices of an Agile Developer almost 4 years ago, and to this day I regularly find myself returning to it time and time again for inspiration. A seminal text by all means, I highly recommend it as a must read for Software Developers of all levels and disciplines.

Some Useful Tips to Keep in Mind

Sunday, February 28th, 2010

Throughout my career I have always been drawn to books which provide a practical way of thinking about software. Books of this nature tend to have an emphasis on fundamental principles which apply to all software engineering disciplines, and form much of the basis of the Agile methodologies many of us have come to appreciate.

Often, I find myself going back to the seminal text; The Pragmatic Programmer as, it provides a great resource for some important things I like to keep in mind from day to day. And so, I just wanted to take a moment to share some of the best tips from the book which I have found to be particularly useful and inspiring.

Care About Your Craft

Why spend your life developing software unless you care about doing it well?

Provide Options, Don’t Make Lame Excuses

Instead of excuses, provide options. Don’t say it can’t be done; explain what can be done.

Critically Analyze What You Read and Hear

Don’t be swayed by vendors, media hype, or dogma. Analyze information in terms of you and your project.

Design with Contracts

Use contracts to document and verify that code does no more and no less than it claims to do.

Refactor Early, Refactor Often

Just as you might weed and rearrange a garden, rewrite, rework, and re-architect code when it needs it. Fix the root of the problem.

Costly Tools Don’t Produce Better Designs

Beware of vendor hype, industry dogma, and the aura of the price tag. Judge tools on their merits.

Start When You’re Ready

You’ve been building experience all your life. Don’t ignore niggling doubts.

Don’t Be a Slave to Formal Methods

Don’t blindly adopt any technique without putting it into the context of your development practices and capabilities.

It’s Both What You Say and the Way You Say It

There’s no point in having great ideas if you don’t communicate them effectively.

You Can’t Write Perfect Software

Software can’t be perfect. Protect your code and users from the inevitable errors.

Build Documentation In, Don’t Bolt It On

Documentation created separately from code is less likely to be correct and up to date.

Put Abstractions in Code, Details in Metadata

Program for the general case, and put the specifics outside the compiled code base.

Work with a User to Think Like a User

It’s the best way to gain insight into how the system will really be used.

Program Close to the Problem Domain

Design and code in your user’s language.

Use a Project Glossary

Create and maintain a single source of all the specific terms and vocabulary for a project.

Be a Catalyst for Change

You can’t force change on people. Instead, show them how the future might be and help them participate in creating it.

DRY – Don’t Repeat Yourself

Every piece of knowledge must have a single, unambiguous, authoritative representation within a system.

Eliminate Effects Between Unrelated Things

Design components that are self-contained, independent, and have a single, well-defined purpose.

Iterate the Schedule with the Code

Use experience you gain as you implement to refine the project time scales.

Use the Power of Command Shells

Use the shell when graphical user interfaces don’t cut it.

Don’t Panic When Debugging

Take a deep breath and THINK! about what could be causing the bug.

Don’t Assume It – Prove It

Prove your assumptions in the actual environment—with real data and boundary conditions.

Write Code That Writes Code

Code generators increase your productivity and help avoid duplication.

Test Your Software, or Your Users Will

Test ruthlessly. Don’t make your users find bugs for you.

Don’t Gather Requirements—Dig for Them

Requirements rarely lie on the surface. They’re buried deep beneath layers of assumptions, misconceptions, and politics.

Abstractions Live Longer than Details

Invest in the abstraction, not the implementation. Abstractions can survive the barrage of changes from different implementations and new technologies.

Don’t Think Outside the Box—Find the Box

When faced with an impossible problem, identify the real constraints. Ask yourself: “Does it have to be done this way? Does it have to be done at all?”;

Some Things Are Better Done than Described

Don’t fall into the specification spiral—at some point you need to start coding.

Don’t Use Manual Procedures

A shell script or batch file will execute the same instructions, in the same order, time after time.

Test State Coverage, Not Code Coverage

Identify and test significant program states. Just testing lines of code isn’t enough.

Gently Exceed Your Users’ Expectations

Come to understand your users’ expectations, then deliver just that little bit more.

Don’t Live with Broken Windows

Fix bad designs, wrong decisions, and poor code when you see them.

Remember the Big Picture

Don’t get so engrossed in the details that you forget to check what’s happening around you.

Make It Easy to Reuse

If it’s easy to reuse, people will. Create an environment that supports reuse.

There Are No Final Decisions

No decision is cast in stone. Instead, consider each as being written in the sand at the beach, and plan for change.

Estimate to Avoid Surprises

Estimate before you start. You’ll spot potential problems up front.

Use a Single Editor Well

The editor should be an extension of your hand; make sure your editor is configurable, extensible, and programmable.

Fix the Problem, Not the Blame

It doesn’t really matter whether the bug is your fault or someone else’s—it is still your problem, and it still needs to be fixed.

“select” Isn’t Broken

It is rare to find a bug in the OS or the compiler, or even a third-party product or library. The bug is most likely in the application.

Learn a Text Manipulation Language

You spend a large part of each day working with text. Why not have the computer do some of it for you?

Use Exceptions for Exceptional Problems

Exceptions can suffer from all the readability and maintainability problems of classic spaghetti code. Reserve exceptions for exceptional things.

Minimize Coupling Between Modules

Avoid coupling by writing shy” code and applying the Law of Demeter.

Design Using Services

Design in terms of services: independent, concurrent objects behind well-defined, consistent interfaces.

Don’t Program by Coincidence

Rely only on reliable things. Beware of accidental complexity, and don’t confuse a happy coincidence with a purposeful plan.

Organize Teams Around Functionality

Don’t separate designers from coders, testers from data modelers. Build teams the way you build code.

Test Early. Test Often. Test Automatically.

Tests that run with every build are much more effective than test plans that sit on a shelf.

Find Bugs Once

Once a human tester finds a bug, it should be the last time a human tester finds that bug. Automatic tests should check for it from then on.

Sign Your Work

Craftsmen of an earlier age were proud to sign their work. You should be, too.

It is my hope that you will find some of these tips helpful and, if so, I suggest keeping those which resonate with you (as well as some of your own) someplace visible for reference.

Why is Programming Fun?

Wednesday, September 16th, 2009

Recently, while re-organizing my bookshelf, I rediscovered a rather inspiring passage that I haven’t read in quite a long time …

The excerpt below is from the book “The Mythical Man-Month: Essays on Software Engineering”, and while the book was originally published in 1974 (before being republished in 1995), I feel it will always remain relevant:

Why is programming fun? What delights may its practitioner expect as his reward?

First is the sheer joy of making things. As the child delights in his mud pie, so the adult enjoys building things, especially things of his own design. I think this delight must be an image of God’s delight in making things, a delight shown in the distinctness and newness of each leaf and each snowflake.

Second is the pleasure of making things that are useful to other people. Deep within, we want others to use our work and to find it helpful. In this respect the programming system is not essentially different from the child’s first clay pencil holder “for Daddy’s office.”

Third is the fascination of fashioning complex puzzle-like objects of interlocking moving parts and watching them work in subtle cycles, playing out the consequences of principles built in from the beginning. The programmed computer has all the fascination of the pinball machine or the jukebox mechanism, carried to the ultimate.

Fourth is the joy of always learning, which springs from the non-repeating nature of the task. In one way or another the problem is ever new, and its solver learns something: sometimes practical, sometimes theoretical, and sometimes both.

Finally, there is the delight of working in such a tractable medium. The programmer, like the poet, works only slightly removed from pure thought-stuff. He builds his castles in the air, from air, creating by exertion of the imagination. Few media of creation are so flexible, so easy to polish and rework, so readily capable of realizing grand conceptual structures.

Yet the program construct, unlike the poet’s words, is real in the sense that it moves and works, producing visible outputs separately from the construct itself. It prints results, draws pictures, produces sounds, moves arms. The magic of myth and legend has come true in our time. One types the correct incantation on a keyboard, and a display screen comes to life, showing things that never were nor could be.

Programming then is fun because it gratifies creative longings built deep within us and delights sensibilities we have in common with all men.

This quote really hits home with me, so I shared it with my team and felt I should also share it with the community, as I imagine it will also inspire many others as well.

Perfectionism, Prudence and Progress

Thursday, June 11th, 2009

Yesterday there was an interesting article on InsideRIA titled: “How much is too much?”. This is a great topic, one which at times I have questioned myself.

Personally, I never take the “easy way out”, preferring to do things the “hard way”, so to speak. At times the benefits in doing things according to best practices, standards and conventions (a.k.a. “the right way”) may not always be immediately obvious. However over the years experience has taught me that in time the benefits always reveal themselves and the pros certainly outweigh the cons.

When given a good amount of forethought to a decision, a design an implementation and so forth a team almost certainly is afforded the ability to continue development feasibly and in a less challenging manner (as opposed to dealing with endless maintenance challenges). When things are done quickly with little regard for anything other than getting working code out the result is always failure at some level, most commonly the maintainability of a product.

With all this in mind it is important to understand that at the end of the day our development efforts, for better or for worse, are simply a means to an end for a specific business need. Therefore just as writing “quick and dirty” code has a negative impact on the business, so too does being a complete perfectionist. Admittedly, this used to be a challenge for me as I would tend to need designs, tests and code to “feel right” for them to be considered production ready, which typically resulted in me working many extra hours on my own time. This in itself is not necessarily a bad thing, but could rather be considered a labor of passion.

Ultimately the goal should be to find just the right balance of perfectionism, prudence and progress, providing necessary trade-offs where appropriate.