In Front of Me

The sun rises in the east. Two parallel lines will never intersect. A true idea must agree with that of which it is the idea. From real-life to mathematics to philosophy, life is full of axioms. Despite their established truth, it takes us time to learn these maxims and become comfortable that they are indeed true.

 

Life is filled with rules to follow, including in writing and reviewing of code

 

Developers build up knowledge of rules and best practices through their experiences writing code. Part of this knowledge is transferred between developers through code reviews, and use of coding patterns. Exposure to different parts of the system will impact the patterns and rules that we learn. When the experience is weighted more towards backend development, as is the case for us, it becomes harder to disseminate UI standards across the group.

 

Last week office discussion has turned to UI code review standards. With our aforementioned skills gap, I’ve had colleagues asking what rules I employ in reviewing of our Angular code. These requests have me pondering if code review best practices really that different in Web UI versus traditional back end services. This week I outline some of the my own personal UI code review regimen, and analyse the overlap between UI and middle tier practices.

 

That’s My Style

 

Common style guides are a great tool for enforcing a consistent code appearance. While developers have their own preferences on attributes such as brace placement and indentation, enforcing a common standard ensures for better code readability. Many organisations, or even individual teams may have their own for key languages within their technology stack. Regardless, there are plenty of external guides that can be leveraged.

 

Despite all engineers having their own style preferences, teams should adopt a common style and stick to it

 

Technology specific ones, such as the Angular Style Guide are preferred over internal standards to align with industry best practices. We have our own Java style guide that integrates with developer IDEs. But for UI, we conform to the Angular Style Guide across the team since we use the Angular framework for all our UI modules. We also ensure our style is enforced using linters such as ESLint for JavaScript and Typescript. Note that it is now standard practice to use ESLint for TypeScript projects as well since TSLint no longer has active support. In addition to providing the aforementioned readability benefits, it also reduces the codebase learning overhead for developers. This applies to those both new to UI within your company, and those new to the organisation altogether.

 

Complex Person

 

File and folder structure standards are another important style guide attribute to note. These are often enforced by frameworks. In our case, these are established by the Angular and Gradle structures mandated on the client and server respectively.

 

For the UI we have all code for a given component housed in a single folder, and common project components listed under a shared folder as outlined in the official Angular Style Guide referenced previously. I also recommend a standard of housing templates in a separate file over inline to ensure consistent usage across the platform. Otherwise programmers tend to use a mixture of inline and separate file based on component size.

 

Modules should use a simple, agreed structure, with a folder for shared components

 

Once projects start to grow in size, the nested structure outlined by Tom Cowley becomes more scalable. Where components are required across modules, they should live in a central module that can be managed using NPM. The balance is difficult to strike, but it’s important to identify copy patterns across your own modules to decide when to split a component into a shared module.

 

Different Languages

 

Developers should also make use of language construct best practices in their code. These attributes are easy to enforce via code reviews. We are well established in server side practice. Through our use of Java developers are comfortable on when to use streams and collectors versus loops.

 

Be mindful of using standard programming language patterns

 

With our heavy use of Typescript, there are some key language good practices that we should be on the lookout for, and should be including in our linter configuration. My biggest bugbear is the use of the any type as general practice. Developers commonly use any in circumstances where they can use a defined type. It is often made use mistakenly in core utilities, where generics would be used in Java. Given Typescript also supports generics, as well as the unknown type, any is not always a valid alternative.

 

Legitimate cases do occur, especially where you are using a third party library interface that is making use of any. Define explicit interfaces and use types to enforce type safety. Make use of Typescript constructs such as union and intersection types where you want to support multiple types. Otherwise, what’s the point of using Typescript over JavaScript?

 

Coming Clean

 

It is a common misconception that best practice patterns are limited to a particular technology or programming language. This notion limits software engineers in their ability to grow as front-to-back developers.

 

Clean code practices can be applied to any programming language, not just Java

 

The key text I refer developers to for code standards is Clean Code by Robert C. Martin. I commonly hear from programmers that it is only considered relevant to Java since all the examples are written in Java. This could not be further from the truth. As part of my own reviews, I am regularly on the lookout for many of the practices Uncle Bob preaches, especially the following:

 

1. Descriptive functional and variable names that gives detail on the constructs purpose. This includes being on the lookout for non-standard abbreviations that may not be clear to all.
2. Levels of nesting. Initially reviewing the indentation of a method can give a strong indication on code complexity. Nesting several levels deep should be refactored to promote readability.
3. Class and function length. Although number of lines of code is not a prescriptive measure of productivity, large classes and methods should be broken down to improve readability.
4. Levels of code duplication. While in UI projects there is an element of duplication in terms of annotation and form component usage. Although it could be argued that using CLI utilities such as Angular CLI does eliminate most of these requirements. Be wary of significant copy pasting efforts. If you are regularly using the same settings on components such as date pickers, or cell for matters, consider extracting them to a shared utility that can be reused across the project, or even modules.

 

There are extensions that are UI specific. A key example for me is the use of variable names in style sheets. Through our use of Sass, we can utilise many CSS extensions such as hierarchy and variables. Use of variables for common style attributes such as colour and padding size are another quality that I look for in reviews. These searches are inspired by the descriptive variable names above, and definitely makes style sheets more readable.

 

Close to the Front

 

The final, crucial element of any UI review is to evaluate the appearance and workflow of any functionality that is built. While wireframes and prototypes should be produced in advance to determine the feature direction, that does not negate the need to assess the appearance.

 

Presentation of any UI component should always conform to consistent practice. As the platform grows, it’s important to replicate style and workflow across the system. UX designers are pivotal in establishing common design. However, in the event that a designer is not present in your team, the responsibility relies on all developers to conform to a standard.

 

Code style is important, but in UI the appearance and workflow of the feature must also be reviewed

 

Centralised components and style sheets allow you to build up reusable facilities to enforce common appearance. It is important to review style as part of any peer review. The simplest way I’ve found is to attach a snapshot to the pull request to showcase the new features appear.

 

While a great initial step, screenshots are limited in their capability to allow inspection of workflow, cross browser compatibility and general style. A better approach would be to have an instance spin up as part of the pre-commit build for every request. These ephemeral instances allow for developers to live test and experiment with the features to provide more targeted feedback. This is currently a dream for us, but very much an achievable one with some time investment.

 

Front Row

 

Regardless of technology, peer reviews are a vital skill for maintaining quality standards and building developer experience. I’ve outlined many of the resources and attributes I use in reviewing of others UI code. The importance is to adopt a common standard to which the team can commit.

 

Growing a set of UI review best practices takes time, and performing of reviews from many other reviewers

 

These maxims will form but the beginning of the total rule set. Our axioms will continue to grow as we receive feedback on our code from others. That includes others reviewing my own code.

 

There is more to reviews that hard rules too. My last, and most important advice, is to be wary of your language in comments, particularly if using commenting features in tools such as GitHub and Bitbucket. In more recent times we’ve had discussions about being clear what items will block merging and which items are optional opinions that can be discussed or actioned later. Unfortunately we have even had discussions about which comments are hurtful and derogatory too (yes really). Feedback builds better engineers. But ensure you communicate them with humanity and empathy.

 

Thanks so much for reading! Do let me know of any review criteria and resources that you also use for reviewing code.