Discussion on: Discussion: It should be hard to write bad code

I feel your pain and frustration, I've been there as well...

I guess some of those issues can be contained and managed with some automation such as linters that set some basic quality standards such as correct formatting, spelling as well as requiring code documentation for exported functions and components.

Linters & Code Complexity Analysis

When a commit is pushed or a PR is submitted then those linters should just run and reject or annotate a contribution that violates those rules, and make the reviewers of the PR aware of that.

Now even with properly formatted and documented code it's still possible to write bad code that is hard to read and maintain. There are some linters out there that perform code complexity analysis and measure "cognitive load". That can be used for preventing deeply nested control flow structures or overly long functions or methods.

Those complexity issues can be usually resolved through refactoring and breaking those big units into smaller units that are often easier to read and verify through unit tests.

Static Code Analysis

Another type of tool is static code analysis that tries to find common types of bugs (e.g. null pointer exceptions / undefined errors). Those analysis tools can be also added to the code review workflow, build process and / or as a git commit hook.

Using TypeScript or Flow

I made the full migration to JS + Flow in a larger React project I'm currently managing.

Proper type hinting is valuable because it adds context to your code that aids your IDE for example with providing you with much better code completion. That speeds up dev time and prevents you from making simple mistakes.

A typical JavaScript problem is dynamic typing, where the type of a variable is derived from the currently assigned value. That can lead to a category of bugs that is completely preventable in statically typed languages, where misuse or incorrect value or parameter assignments are caught at compile time.

Flow for example also allows you to declare your own data types. You can use that for mirroring API data types, and your IDE provides you with code completion for those object fields.

Type hints add a lot of clarity to the code base:

export interface User {
    id: number,
    name: string
}

export type UserCardProps = {
    user: User
};

export function UserGreeting(props: UserCardProps) {
    const {user} = props;
    // At this point your IDE knows that `user` is of type `User`

    return <h1>Hello {user.name}</h1>;
}

Type hinting can be enforced and verified with linters btw, so also that can be automated to improve the quality of new contributions.

Team Discipline and Establishing New Standards

It's mostly up to team discipline and communicating to the higher ups that hard to understand, poorly tested, poorly documented code will cost them in the long run. The team needs to decide to tackle this and dedicate their time to source code quality control.

Doing Proper Code Review

Your team should also take the code review process seriously and try to discuss ways to improve the contribution in a constructive manner.

I've been part of code review processes were reviewers basically just go through the motions and give it the green light without even leaving a comment behind. I've been guilty of that, too. It's understandable because it takes time to do proper code review. But it's time well spent for the sake of the code base and overall health of the system the team is working on. It should be simply seen as part of the development job.

It's probably also a team / company culture problem. Reviews usually contain statements and corrections that can be taken personally by the contributor. And some reviewers might even make it personal. So it's important to turn the process into something friendly that is thorough nevertheless. There are numerous code review checklists on the Internet that can be taken as guidelines.

Unit Tests Document Expected Behavior

I think of unit tests less as a verification tool that the code base is bug free. That's pretty much impossible to know for sure. I see it more as a documentation for the expected behavior of the software. They should go through expected scenarios and their inputs, and verify that the expected output is produced by the tested code. Tests should have self-describing names that briefly summarize each scenario.

Your team could require that each contribution must come with unit tests. Especially for bug fixes, to make sure that the bug does not occur again.

I usually write my unit tests first to reproduce the bug on my local. Once that is taken care of, I fix the tested code until it passes my tests.

Testable Code Is Often Better Code

Writing unit tests has often a positive effect on the quality of the tested code. It nudges you towards writing code that is more easily testable in the first place. More easily testable code is also often more readable and easier to maintain.

Writing testable code typically requires you to decouple your business logic from framework dependencies that are usually complicated to set up in unit tests.

You would write dedicated functions that do one thing, and one thing only, which can be more easily tested. Decoupling your code from those dependencies also allows you to mock those dependencies more easily if they are needed for running your tests.

Slow but steady

Once better procedures are in place and enforce them through proper code review, and by using tools such as linters, code analysis, unit tests etc., you have at least some extra assurance that new changes are less likely to make things worse.

From there it would be basically a slow iterative process to get the rest of the code base up to speed, and I guess that would come naturally when new features need to be added to those older components, or bugs need to be fixed.