iTranslated by AI

The content below is an AI-generated translation. This is an experimental feature, and may contain errors. View original article
🍀

17 Tips for Writing Good Code: A Comprehensive Summary

に公開
6

About this article

I am a web application engineer at a company that provides web infrastructure.
I am often responsible for creating aggregations (BFF) that combine multiple backend APIs—such as infrastructure and customer management platforms—and implementing the frontend.
I frequently choose TypeScript and C# as my languages, and use React.js and Vue.js for the frontend. Against this background, I will post some tips that I keep in mind daily to write "good code."

What I won't cover

  • Basic content such as indent alignment, naming fundamentals, and access modifiers are omitted.
  • The code is written in TypeScript, but the content is not specialized for a specific language.
  • The content covers programming in general and is not specialized for a specific layer like frontend or backend.

What is Good Code?

The definition of "good code" here is as follows:

  • High maintainability
  • High readability

I looked up the meaning of each on Wikipedia.

Maintainability

The ease with which software defects such as bugs can be fixed, characteristics like performance or usability can be improved, or functions not originally planned can be added or changed with minimal effort and cost.

Readability

Refers to how easy it is for a human to understand the purpose and flow of a program's source code when reading it.

While I think maintainability is a concept that encompasses readability, I will separate them here.

Table of Contents

  1. Write comments starting with the background
  2. Use summary variables
  3. Use explanatory variables
  4. Extract into modules
  5. Return early
  6. Outsource cross-cutting concerns with AOP
  7. Use immutable types
  8. Encapsulation
  9. Hide legacy code
  10. Cohesion and Coupling
  11. Basically avoid using inheritance
  12. Don't force commonality
  13. Dependency Injection (DI)
  14. Dependency Inversion Principle (DIP)
  15. Use Generics
  16. Refer to architectures
  17. Rely on automatic generation

Write comments starting with the background

I occasionally see comments that are simply the implementation translated into Japanese, like the following:

mm..🤔
// Execute if master role, or if isOldTypeAccount=true and admin role
if (user.role === "master" || (user.isOldTypeAccount && user.role === "admin")) {
  // Processing allowed only for admin users
}

With this, anxieties and questions remain, such as: What is isOldTypeAccount? What is the admin role? Do I need to consider this in future modifications?

good😊
// Execute only for administrator roles
// Accounts from before the service renewal (isOldTypeAccount=true) used "admin" for the administrator role
if (user.role === "master" || (user.isOldTypeAccount && user.role === "admin")) {
  // Processing allowed only for admin users
}

By describing the background, the reason why "master role" and "admin role" coexist and why the isOldTypeAccount flag was created becomes clear.

The implementer and the reader have different sets of underlying knowledge. We should take this into account when writing comments (and implementation).

Use summary variables

From the code used earlier, let's turn the part "Accounts from before the service renewal (isOldTypeAccount=true) used 'admin' for the administrator role" into a summary variable.

good+😊
const isOldTypeMasterRole = user.isOldTypeAccount && user.role === "admin";
if (user.role === "master" || isOldTypeMasterRole) {
  // Processing allowed only for admin users
}

By doing the above, the comment is no longer necessary. It is even better if you extract it into a module.

good++😊
if (isMasterRoleUser(user)) {
  // Processing allowed only for admin users
}

Use explanatory variables

To obtain a target value, you might reference deep layers of a class or perform risky string operations. Naively processing such responses looks poor and reduces readability.

mm..🤔
class UserGetService {
  public get(userId: string) {
    const api = new LegacyApi()
    const resp = api.getUsers(userId)
    return {
      userId: resp.data.users[0].id,
      lastName: resp.data.users[0].info.fullName.split(" ")[0],
      firstName: resp.data.users[0].info.fullName.split(" ")[1],
    }
  }
}

In this case, defining explanatory variables can be expected to improve readability.

good😊
class UserGetService {
  public get(userId: string) {
    const api = new LegacyApi()
    const resp = api.getUsers(userId)

    const user = resp.data.users[0]
    const userNames = user.info.fullName.split(" ")
    const lastName = userNames[0]
    const firstName = userNames[1]

    return { userId: user.id, lastName, firstName }
  }
}

Extract into modules

Let's take a user creation module as an example. In the bad example, the validation logic for userId and password becomes noise, which decreases readability.

mm..🤔
  function createUser(userId: string, password: string) {
    // Validation for userId
    if (userId && userId.length >= 8 && userId.length <= 32) {
      // Validation for password
      if (password && password.length >= 8 && password.length <= 64) {
        const api = new UserApi(userId, password)
        api.create(userId, password)
      }
    }
  }

Taking userId as an example, the following three checks are performed:

  • Does userId have a value?
  • Is userId 8 characters or longer?
  • Is userId 32 characters or shorter?

These handle the concern of "whether userId is the expected value." When concerns align, they can be extracted as a separate module.

good😊
  function createUser(userId: string, password: string) {
    if (isUserIdValid(userId)) {
      if (isPasswordValid(password)) {
        const api = new UserApi(userId, password)
        api.create(userId, password)
      }
    }
  }
  function isUserIdValid(value: string) {
    return value && value.length >= 8 && value.length <= 32
  }
  function isPasswordValid(value: string) {
    return value && value.length >= 8 && value.length <= 64
  }

By extracting it into a module and naming it, you can now understand what concern is being processed at a glance. Before supplementing logic with comments, it's good to consider whether that logic should be extracted as a module.

Return early

The primary advantage of using early returns is the ability to reduce nesting. Readability decreases as nesting becomes deeper.

mm..🤔
  function createUser(userId: string, password: string) {
    if (isUserIdValid(userId)) {
      if (isPasswordValid(password)) {
        const api = new UserApi()
        api.create(userId, password)
      }
    }
  }

Logic that is not the essence of the process—for example, validation before executing an API or recording to a database—should be returned early.

good😊
  function createUser(userId: string, password: string) {
    if (!isUserIdValid(userId)) { return }
    if (!isPasswordValid(password)) { return }
    const api = new UserApi()
    api.create(userId, password)
  }

Compared to the bad example, the good example reduces nesting and improves the visibility of the essential code.

Outsource cross-cutting concerns with AOP

The terminology varies by language—annotations in Java, attributes in C#, decorators in JavaScript—but I recommend using them.

For example, if authentication or logging logic is mixed into a user creation module, it becomes noise relative to the class's main concern and reduces readability.
Such processing should be outsourced so that readers can focus on the essential logic.

mm..🤔
  function createUser(userId: string, password: string) {
    this.logger.log(Level.info, "start user create")
    if (!this.auth()) { throw new UnauthenticatedError() }

    if (!isUserIdValid(userId)) { return }
    if (!isPasswordValid(password)) { return }
    const api = new UserApi()
    api.create(userId, password)

    logger.log(Level.info, "end user create")
  }
good😊
  @auth()
  @log("user create")
  function createUser(userId: string, password: string) {
    if (!isUserIdValid(userId)) { return }
    if (!isPasswordValid(password)) { return }
    const api = new UserApi()
    api.create(userId, password)
  }

Use immutable types

If the exposed fields are mutable, you always need to watch whether they have been changed somewhere during their lifetime.
Declaring them in an immutable state releases working memory (the ability to temporarily store and process information needed for tasks and operations), making the code easier to read.

class User {
-  public userId: string
+  public readonly userId: string
}

Private fields should also be made immutable as much as possible.
The information that something is "unwritable" becomes more effective as the amount of code increases.

class User {
  public readonly userId: string
-  private password: string
+  private readonly password: string
}

In languages where class initialization can be restricted by constructors, it is most desirable that the state of the class cannot be changed except through the constructor's arguments.
With the knowledge that "fields can only be changed in the constructor," that instance can be reused without worry, even if it gets buried in a vast amount of code.

class CreateUserCommand {
  public readonly userId: string
  private readonly password: string
+  constructor(userId: string, password: string) {
+    this.userId = userId
+    this.password = password
+  }
  public execute() { ... }
}

If the argument is a reference type, it is desirable to make the argument immutable as well.
This guarantees that the argument is not modified inside the method (it's not a destructive method), eliminating the need to check the method's internals and strengthening encapsulation.

class CreateUserCommand {
  public readonly userId: string
  private readonly password: string
-  constructor(user: { userId: string; password: string }) {
+  constructor(user: Readonly<{ userId: string; password: string }>) {
    this.userId = user.userId
    this.password = user.password
  }
  public execute() { ... }
}

Summary of immutable types:

  • Make fields readonly
  • Do not expose setter (do not allow external changes)
  • Utilize constructor restrictions
  • Declare arguments as immutable types

Encapsulation

Interpretations of encapsulation may vary from person to person.
I consider encapsulation to mean "being able to use it correctly without having to check the implementation."

"Being able to use it correctly without having to check the implementation" means that you can understand what kind of module it is just by looking at the class definition.

class Register {
  public userId: string
  public password: string
  public execute() {
    if (!this.userId) { throw Error("Please provide a userId") }
    if (!this.password) { throw Error("Please provide a password")  }
    // User creation process
  }
}

When a class like the one above exists, extracting the class definition looks like this:

class Register {
  userId: string
  password: string
  execute(): void
}

Looking only at this, information is missing, and it's unclear how to use the class.
You would need to open the implementation and check the inside of the class to use it.

First, change the class name. Give it a specific name that makes it clear what responsibilities the class has. Abstract names are the first step toward a GOD class.

- class Register {
+ class UserCreateCommand {
  public userId: string
  public password: string
  public execute() {
    if (!this.userId) { throw Error("Please provide a userId") }
    if (!this.password) { throw Error("Please provide a password")  }
    // User creation process
  }
}

By correcting the class name, it is now understood to be a class that creates users.

Even with this, the information that userId and password are required is still not apparent.

Rather than explicitly stating that "userId and password are required," we use a constructor to restrict instantiation so that it cannot be done without passing userId and password.

class UserCreateCommand {
-  public userId: string
-  public password: string
+  constructor(public userId: string, public password: string) { }
  public execute() {
    if (!this.userId) { throw Error("Please provide a userId") }
    if (!this.password) { throw Error("Please provide a password")  }
    // User creation process
  }
}

Since we changed it to receive parameters in the constructor, it is desirable to perform validation in the constructor as well.

class UserCreateCommand {
  constructor(public userId: string, public password: string) {
+    if (!this.userId) { throw Error("Please provide a userId") }
+    if (!this.password) { throw Error("Please provide a password")  }
  }
  public execute() {
-    if (!this.userId) { throw Error("Please provide a userId") }
-    if (!this.password) { throw Error("Please provide a password")  }
    // User creation process
  }
}

I think it has become much easier to use at this stage, but finally, we add the readonly constraint to the fields. This guarantees that the fields are not changed outside of the constructor.

class UserCreateCommand {
-  constructor(public userId: string, public password: string) {
+  constructor(public readonly userId: string, public readonly password: string) {
    if (!this.userId) { throw Error("Please provide a userId") }
    if (!this.password) { throw Error("Please provide a password")  }
  }
  public execute() {
    // User creation process
  }
}

Extracting the final class definition looks like this:

class UserCreateCommand {
  readonly userId: string
  readonly password: string
  constructor(userId: string, password: string)
  execute(): void
}

You can now understand how to use it just by looking at it from the outside.

Hide legacy code

When dealing with legacy code or legacy APIs, you might encounter implementations like a "catch-all data class" with a large number of optional arguments. These control behavior depending on how the arguments are passed.

The following LegacyApi controls the creation of regular users and corporate users based on how values are filled into the User type. This API is not encapsulated, and you would likely end up having to ask the person in charge of the API how to use it.

mm..🤔
type User = {
  name: string
  mail: string
  address?: string // Mandatory only for regular users
  corporateUser?: boolean // "true" for corporate users
  corporateAddress?: string // Mandatory only for corporate users
}

class UserCreateFacade {
  public createUser() {
    const api = new LegacyApi()
    const user: User = {
      name: "yamada taro",
      mail: "yamada@xxx.com",
      address: "Kanagawa Prefecture, Kamakura City...",
      // Are the parameters sufficient...??
    }
    api.createUser(user)
  }
  public createCorporateUser() {
    const api = new LegacyApi()
    const user: User = {
      name: "Good Codes Inc.",
      mail: "good_codes@xxx.com",
      corporateUser: true,
      corporateAddress: "Tokyo, Shinagawa Ward...",
      // Is address also required...??
    }
    api.createUser(user)
  }
}

To ensure that members of the same team don't have to ask the person in charge of the API twice, it is good to define the generation of parameters in a Factory class.

good😊
type User = {
  name: string
  mail: string
  address?: string // Mandatory only for regular users
  corporateUser?: boolean // "true" for corporate users
  corporateAddress?: string // Mandatory only for corporate users
}

class UserFactory {
  public static getUser(
    name: string,
    mail: string,
    address: string
  ): User {
    return { name, mail, address }
  }
  public static getCorporateUser(
    name: string,
    mail: string,
    corporateAddress: string
  ): User {
    return { name, mail, corporateAddress, corporateUser: true }
  }
}

class UserCreateFacade {
  public createUser() {
    const api = new LegacyApi()
    const user = UserFactory.getUser(
      "yamada taro",
      "yamada@xxx.com",
      "Kanagawa Prefecture, Kamakura City..."
    )
    api.createUser(user)
  }
  public createCorporateUser() {
    const api = new LegacyApi()
    const user = UserFactory.getCorporateUser(
      "Good Codes Inc.",
      "good_codes@xxx.com",
      "Tokyo, Shinagawa Ward..."
    )
    api.createUser(user)
  }
}

Cohesion and Coupling

High Cohesion

Let's take a service class that creates a user and then registers an operation history as an example.

type InputData = { userId: string; password: string }

class CreateUserService {
  private readonly userApi = new UserApi()
  private readonly historyApi = new HistoryApi()
  public handle(inputData: Readonly<InputData>) {
    this.userApi.create(inputData.userId, inputData.password)
    this.historyApi.add(inputData.userId, "new user created.")
  }
}

class UserApi {
  create(userId: string, password: string) {}
}
class HistoryApi {
  add(userId: string, message: string) {}
}

In the above class, all fields used by handle() are within the same class, and the relevance between fields and methods is high. Classes or modules where processing and data are located close to each other like this are said to have high cohesion.

Loose Coupling

Currently, UserCreateService is implemented depending on the UserApi and HistoryApi classes. We will change each of these to interfaces to eliminate class dependencies.

type InputData = { userId: string; password: string }

class CreateUserService {
  constructor(
    private readonly userApi: UserApi,
    private readonly historyApi: HistoryApi
  ) {}
  public handle(inputData: Readonly<InputData>) {
    this.userApi.create(inputData.userId, inputData.password)
    this.historyApi.add(inputData.userId, "new user created.")
  }
}

// Change from class to interface
interface UserApi {
  create(userId: string, password: string): void
}
interface HistoryApi {
  add(userId: string, message: string): void
}

By making the implementation depend on interfaces in this way, UserCreateService has become an independent class with no dependencies on other specific classes. Such classes or modules are said to have low coupling (loose coupling).

Summary of High Cohesion and Loose Coupling

High-cohesion, loose-coupling classes have many benefits, such as:

  • High readability
  • High maintainability (easy to modify because the impact is localized)
  • Less likely to have conflicts in modified parts
  • Easy to create test code

To put it extremely, if something is built with a high-cohesion and loose-coupling structure, you can manage somehow. I believe it is that important of an element for maintainability.

Basically avoid using inheritance

Inheritance, if not used properly, tends to result in low-cohesion, tightly-coupled GOD classes. I often see programs where complexity is increased by creating common logic in base classes. If you want to achieve commonality, extracting it into a separate class works better in most cases.

mm..🤔
public class Base {
  public common() {}
}
public class MyClass extends Base {
  public method() { 
    this.common()
  }
}
good😊
public class Shared {
  public common() {}
}
public class MyClass {
  public method() { 
    const shared = new Shared()
    shared.common()
  }
}

Don't force commonality

When I first started learning programming, I used to think that writing fewer lines of code was the ultimate goal, and I often overdid DRY (Don't Repeat Yourself). Even if forcing extraction of things with different contexts works well at the time of release, it leads to trouble later during maintenance. Eventually, you end up decomposing them back into separate modules or adding conditional branches...

There's more to think about when modifying existing code than when creating something new.

For example, "running" for a human and "running" for an animal might be fine as the same process at first. However, there is a high possibility that features will be added only for humans later. If the process is shared, you'll need to consider and verify modifications for both "running human" and "running animal."

Even if the processes happen to be the same now, if they are essentially different, it's important not to force commonality and to allow redundant code.

Dependency Injection (DI)

Using DI to eliminate dependencies between classes offers many benefits.
Here, I would like to highlight two characteristics: "improved readability" and "interchangeability."

Improved Readability

Take the process of creating a user as an example. The requirements are as follows:

  1. Start log
  2. Create user
  3. End log
  • Logs can be switched between text output and console output.
  • The user creation process can be changed for testing, development, and production.

When not using DI

mm..🤔
class CreateUserHandler {
  public handle() {
    const logger =
      config.logger === "console" ? new ConsoleLogger() : new TextLogger();

    logger.info("start create user.");

    if (config.env === "test") {
      const service = new CreateUserServiceTest();
      service.handle();
    } else if (config.env === "development") {
      const service = new CreateUserServiceDev();
      service.handle();
    } else {
      const service = new CreateUserServiceProd();
      service.handle();
    }

    logger.info("end create user.");
  }
}
mm..🤔 Other code
const config = {
  env: "production",
  logger: "text",
};

class CreateUserServiceTest {
  public handle() {}
}
class CreateUserServiceDev {
  public handle() {}
}
class CreateUserServiceProd {
  public handle() {}
}

class ConsoleLogger {
  public info(msg: string) {}
}
class TextLogger {
  public info(msg: string) {}
}

As you can see, conditional branches unrelated to the logic are mixed into CreateUserHandler as noise, making the process difficult to follow.

According to the concept of cyclomatic complexity, the complexity increases with the number of conditional branches.

When using DI

good😊
class CreateUserHandler {
  // Minimal implementation for this example
  // In libraries, injection is often done using decorators or constructors
  private readonly logger = diContainer.get<ILogger>("Logger");
  private readonly createUserService = diContainer.get<ICreateUserService>("CreateUserService");

  public handle() {
    this.logger.info("start create user.");
    this.createUserService.handle();
    this.logger.info("end create user.");
  }
}
good😊 Other code
const config = {
  env: "production",
  logger: "text",
};

interface ICreateUserService {
  handle();
}

interface ILogger {
  info(msg: string);
}

class CreateUserServiceTest implements ICreateUserService {
  public handle() {}
}
class CreateUserServiceDev implements ICreateUserService {
  public handle() {}
}
class CreateUserServiceProd implements ICreateUserService {
  public handle() {}
}

class ConsoleLogger implements ILogger {
  public info(msg: string) {}
}
class TextLogger implements ILogger {
  public info(msg: string) {}
}

class PoorDiContainer {
  private readonly container: { [key: string]: unknown } = {};
  public add(key: string, instance: unknown) {
    this.container[key] = instance;
  }
  public get<T>(key: string): T {
    return this.container[key] as T;
  }
}

// Set up dependencies in the DI container outside of the logic
const diContainer = new PoorDiContainer();

if (config.env === "test") {
  diContainer.add("CreateUserService", new CreateUserServiceTest());
} else if (config.env === "development") {
  diContainer.add("CreateUserService", new CreateUserServiceDev());
} else {
  diContainer.add("CreateUserService", new CreateUserServiceProd());
}

if (config.logger === "console") {
  diContainer.add("Logger", new ConsoleLogger());
} else {
  diContainer.add("Logger", new TextLogger());
}

By handling class assignment via config during registration with diContainer, the implementation of CreateUserHandler has become simple.
While the amount of "other code" increased with interface definitions and such, if encapsulation is handled properly, you only need to check the interface parts, so the amount of boilerplate code isn't a concern.

Swapping for convenient implementations based on the development phase

In test code execution, you don't want to include processes with side effects.
In this case as well, you can switch to a test implementation just by changing the config, without needing to write test-specific code inside CreateUserHandler.

CreateUserHandler.spec.ts
const config = {
  env: "test", // Switch to a side-effect-free implementation for testing
  logger: "console", // Switch to a side-effect-free implementation for testing
};

describe("User Creation", () => {
  it("should complete normally", () => {
    const createUserHandler = new CreateUserHandler();
    createUserHandler.handle();
  });
});

If you were to add a new Logger, the only place you'd need to change is where you add it to the DI container.

// Assuming configuration referenced by the DI container
const config = {
  env: "production",
  logger: "mongodb",
}

if (config.logger === "mongodb") { // new!!
  diContainer.add("Logger", new MongoDbLogger())
} else if (config.logger === "console") {
  diContainer.add("Logger", new ConsoleLogger())
} else {
  diContainer.add("Logger", new TextLogger())
}

Using DI in this way thins the dependencies between classes, creates a set of highly independent modules, and makes it possible to easily switch between implementations suited for each development phase.

In actual production, it's best to use libraries known as DI containers to manage dependencies. They come with features for using DI more comfortably, such as instance caching and instance generation via the factory pattern.

Dependency Inversion Principle (DIP)

This is one of the SOLID principles, known as the "Dependency Inversion Principle" in translation.
A well-known design that utilizes DIP is the Repository pattern.
By making higher-level modules depend on interfaces and switching lower-level modules through DI, you can decouple persistence-related logic from business logic, effectively abstracting data-related operations.

By inverting dependencies in this manner, it becomes unnecessary to modify the implementation of critical domain logic to suit the requirements of lower-level modules.
The concept of placing the domain part at the center of the system and pushing frequently changing layers to the outside is a core tenet adopted in architectures like Onion Architecture and Clean Architecture.

Use Generics

Generics are straightforward once you are used to them, but for beginners, they represent a new concept that can be challenging to understand initially.

Furthermore, while using them might feel intuitive, designing and implementing modules that leverage generics requires a different set of skills.
For this reason, I consider them a "rite of passage" for beginners working with statically typed languages.

Depending on the language, generics can be used to avoid boxing, prevent runtime errors caused by casting, and increase the expressiveness of interfaces, making them extremely useful.
Since the specific type can be designated by the caller, you can create modules that are highly versatile while maintaining processing consistency.

List without type specification

mm..🤔
class AnyList {
  private readonly source: any[] = []
  public add(value: any) {
    this.source.push(value)
  }
  public get(index: number): any {
    return this.source[index]
  }
}

const anyList = new AnyList()
anyList.add("xxx")

const value = anyList.get(0) // It is of type 'any' at this point
if (typeof value === "string") {
  // It is treated as a string type here
}

Taking TypeScript as an example, the value is treated as an any type until it is narrowed down by a type guard.
As a result, you must always be aware of the specific type during usage.
Additionally, if you inadvertently cast to the wrong type, it will lead to a runtime error, necessitating constant vigilance.

List using Generics

good😊
class GenericsList<T> {
  private readonly source: T[] = []
  public add(value: T) {
    this.source.push(value)
  }
  public get(index: number): T {
    return this.source[index]
  }
}

const genericsList = new GenericsList<string>()
genericsList.add("xxx")

const value: string = genericsList.get(0) // It is of string type at this point

By using generics and allowing the caller to define the type, we have created a class that can handle any type, whether it be string, number, or an object.

Refer to architectures

Architectures are great inventions from our predecessors. There is no reason not to refer to them.

I will cover the following here. There are many dedicated articles for details, so please refer to those.

  • Clean Architecture, Onion Architecture
  • CQS and CQRS

Clean Architecture, Onion Architecture

I recognize that the most important concept common to both is "directing dependencies toward the center of the circle."

This is achieved by using things like the Repository pattern to invert dependencies.
Allowing the direction of dependencies only from the outside in and keeping business logic at the center—by adhering to these, you prevent business logic from leaking into other layers, and the domain part becomes independent.

When the most critical business logic is independent, other layers become interchangeable (pluggable) parts, and the entire program becomes easy to test and highly maintainable.

This article is very helpful.
https://qiita.com/nrslib/items/a5f902c4defc83bd46b8

CQS and CQRS

These come from the idea that since the concerns for fetching data (Query) and changing data (Command) are different, separating Query and Command works better than simple CRUD.

Separating Q and C within a program is called CQS, and separating them at the server or resource level is called CQRS.

Event Sourcing is often used alongside server-level separation.
Event Sourcing treats use cases as events and accumulates them in the database. Even a use case corresponding to Delete is inserted into the DB as a deletion event (similar to how Git works).
The accumulated events are called an event journal, which serves as a data source where inserts are performed only from the Command side.

Conversely, on the Query side, a database serving as a read model is created using convenient data extracted from the event journal.
A convenient data structure offers high performance during retrieval and is highly expressive.
The read model maintains the latest state by monitoring transaction logs or detecting changes in the data source through database change hooks (*1) and applying updates.

*1 This refers to things like DynamoDB Streams or Oracle's CHANGE NOTIFICATION.

Rely on automatic generation

When humans write code, their individual philosophies enter the code. Sometimes this can reduce readability or cause confusion.
For example, things like API wrappers or entities corresponding to database schemas are easier for consumers to use when they are created mechanically.

While database schema scaffolding has a long history, it is also possible to automatically generate a series of API request codes by following specifications such as OpenAPI, GraphQL, or gRPC.

I often use code generation with OpenAPI, so I will describe that example. In my work, the backend API is often .NET, and the frontend often uses React.js or Vue.js.
In this case as well, I automatically generate an SDK that wraps the request processing for each endpoint from the OpenAPI definition file (YAML) created at the backend using openapi-generator.

Even if the endpoint definition changes, you just need to regenerate it, so there is no maintenance cost. Furthermore, despite using different languages like C# and TypeScript, we can achieve type-safe implementation through the OpenAPI definition file.

Additionally, in our company's system, there are layers even lower than the backend API mentioned above, and they are also connected via APIs. Since those APIs also publish OpenAPI definition files, the consumers generate SDKs from those definition files.

In this article, I describe how to achieve type safety by automatically generating an SDK from an OpenAPI definition file using .NET for the backend and TypeScript for the frontend, so please take a look if you are interested.
https://qiita.com/ishiyama0530/items/da56d1e26fbdc72b9486

Summary

These were the tips I keep in mind daily for writing "good code."
In my environment, these apply very well. But how about yours?
Recently, many reference books on OOP and design have been published. However, even if you try to merge the contents of those books into your own work environment, it often doesn't go well.
This is because the systems the authors create as samples are completely different from your system in terms of requirements and environment.
In the end, I believe that to write good code, you have to build up many "drawers" of knowledge, repeatedly use trial and error to see how to incorporate them into your environment without causing conflicts, and accumulate study and experience.
I also continue to take in new information every day, wanting to write better code next time than I do now.
I intend to update this article whenever I can verbalize more good tips.

This turned out to be a long post and ended up a bit like a poem. lol
To those who read this, thank you for your precious time. 🙌

CHANGELOG

  • 2022-12-18
    • Inverted the conditional branch in "# Write comments starting with the background" (judged from received comments that early returns were difficult to understand in that context)
    • Refactored field name from "id" to "userId"
    • Refactored method name from "validateXxx" to "isXxxValid"
    • Fixed class syntax errors

Discussion

misukenmisuken

早期リターンの改善後のコードが以下になってますが、if文の判定に!が足りていないように思います。

  function createUser(id: string, password: string) {
    if (validateId(id)) { return }
    if (validatePassword(password)) { return }
    const api = new UserApi()
    api.create(id, password)
  }
ishiyamaishiyama

ほんとですね!
教えてくれてありがとうございます。取り急ぎ修正しました。
今思うと「validateXX」は曖昧な名前でしたね。(しっかり自分で引っかかってしまいました😅)

あいや - aiya000あいや - aiya000

DeepReadonlyを定義して、ReadonlyにDeepReadonlyしたり、
eslintとprettierで自動フォーマットするとよさそう🙆‍♂

ishiyamaishiyama

DeepReadonly初めて聞きました!(ありがとうございます!!)
調べてみたら下記のリポジトリが便利そうなので、今度眺めてみようと思います👀

https://github.com/ts-essentials/ts-essentials

eslintとprettierで自動フォーマットするとよさそう🙆‍♂

ですね!実際にTypeScriptで運用するときはどちらも組み込むようにしています🙆‍♂️

tarupotarupo

突然ですみません、第一目のコードなんですが、ここ間違っているじゃないでしょうか?

// ロールがmasterでなく、adminでもなくisOldTypeAccountでもない場合エラー
if (user.role !== "master" || user.role !== "admin" || !user.isOldTypeAccount) {
  throw new Error("管理者ユーザーのみ許可されています。");
}

ここ|| ではなく、 &&なのでは?

// ロールがmasterでなく、adminでもなくisOldTypeAccountでもない場合エラー
if (user.role !== "master" && user.role !== "admin" && !user.isOldTypeAccount) {
  throw new Error("管理者ユーザーのみ許可されています。");
}

日本語は下手なので、誤解したらすみません。

ishiyamaishiyama

ここ、、サンプルコードが良くなかったですね。
早期リターンを例にせず、以下のようにしたほうが伝わりやすかったです。

if (user.role === "master" || (user.role === "admin" && user.isOldTypeAccount)) {
  // 管理者ユーザーのみ許可されている処理
}

早期リターンにする場合だと以下だけで条件クリアしているのでisOldTypeAccountの判定は不要ですものね。

if (user.role !== "master" || user.role !== "admin") {  // || !user.isOldTypeAccount) {
  throw new Error("管理者ユーザーのみ許可されています。");
}

ご指摘ありがとうございます!
時間あるときに修正したいと思います🙆‍♂️