Functions over comments

Code reads different than human languages. When we don't understand what the code is doing, we write comments in our code in the form of human languages. But the language we speak and write every day can be bloated or ambiguous. The human brain is just really good at extracting context. When you are trying to understand what a snippet of code does, a story is rarely the best method. However, if the confusing code is wrapped in properly named functions, half of our questions are already answered.

What does this code do?

The following code is part of a REST endpoint where there was a bug:

async function getOrder(id) {
    try {
        const order = await ORM.queryBuilder
            .select()
            .from("orders")
            .where("id", id)
            .first();

        if (!order) {
            return {
                statusCode: 404,
                response: { message: "Order not found" }
            };
        }

        const userZipcode = order.user.zipcode;
        const taxRates = await ORM.queryBuilder
            .select()
            .from("tax_rates")
            .where("zipcode", userZipcode);

        const subtotal = order.total + order.shippingCost;
        let taxes = 0;
        for (const taxRate of taxRates) {
            taxes += (subtotal * taxRate.rate) / 100;
        }

        const response = {
            orderId: order.id,
            subtotal: subtotal,
            taxes: taxes,
            total: subtotal + taxes
        };

        return {
            statusCode: 200,
            response: response
        };
    } catch (error) {
        console.error("Error fetching order:", error);
        return {
            statusCode: 500,
            response: { message: "Internal Server Error" }
        };
    }
}

When there is an error in this code, you have to load the entire api in your head to debug. The first step into debugging or trying to resolve an issue is to ask questions to help us focus our effort. What is this code doing in the first place? I've seen this many times, developers try to document their code by adding one line comments to say what the code is actually doing. Ex:

async function getOrder(id) {
    try {
        // Fetch order from the database using query builder
        const order = await ORM.queryBuilder
            .select()
            .from("orders")
            .where("id", id)
            .first();

        // If order not found, return 404
        if (!order) {
            return {
                statusCode: 404,
                response: { message: "Order not found" }
            };
        }

        // Get user's zipcode from the order
        const userZipcode = order.user.zipcode;

        // Fetch tax rates based on user's zipcode using query builder
        const taxRates = await ORM.queryBuilder
            .select()
            .from("tax_rates")
            .where("zipcode", userZipcode);

        // Calculate subtotal including shipping
        const subtotal = order.total + order.shippingCost;

        // Calculate taxes based on subtotal and tax rates
        let taxes = 0;
        for (const taxRate of taxRates) {
            taxes += (subtotal * taxRate.rate) / 100;
        }

        // Prepare response JSON
        const response = {
            orderId: order.id,
            subtotal: subtotal,
            taxes: taxes,
            total: subtotal + taxes
        };

        // Return success response with JSON and 200 status code
        return {
            statusCode: 200,
            response: response
        };
    } catch (error) {
        // Handle any errors and return appropriate response
        console.error("Error fetching order:", error);
        return {
            statusCode: 500,
            response: { message: "Internal Server Error" }
        };
    }
}

This looks useful, but it doesn't do much more than labeling part of the code. In fact it is redundant and can easily become outdated when part of the code changes. Developers may be good at updating code, but I've seen my fair share of obsolete comments.

So how can we approach debugging this code? Let's first ask a few questions:

• Is the error in fetching the order?
• Is the error in getting the zip code?
• Is the error in getting the correct tax rate?
• Is there an error in calculating the final tax rate?

The function does too much to easily narrow it down. For example, if you want to test how the tax rate is calculated, you have to make sure you are loading an order and a user, and have a working zipcode. It adds to the cognitive load.

My solution is to do away with comments, make the code more readable, and make it testable. Comments will only be used where there is exceptional behavior. Let's turn the sections of the code that we want to independently test into functions.

async function getOrder(orderId) {
    try {
        const order = getUserOrderById(orderId);
        if (!order) {
            return {
                statusCode: 404,
                response: { message: "Order not found" }
            };
        }

        const userZipcode = order.user.zipcode;

        // if the zipcode does not have taxrates the default rates
        // will be returned.
        const taxRates = getZipcodeTaxRates(userZipcode);

        const subtotal = calculateOrderSubtotal(order);

        const taxes = calculateSubtotalTaxes(subtotal, taxRates);

        const response = {
            orderId: order.id,
            subtotal: subtotal,
            taxes: taxes,
            total: subtotal + taxes
        };

        return {
            statusCode: 200,
            response: response
        };
    } catch (error) {
        console.error("Error fetching order:", error);
        return {
            statusCode: 500,
            response: { message: "Internal Server Error" }
        };
    }
}

We've introduced these functions:

• getUserOrderById(orderId);
• getZipcodeTaxRates(userZipcode)
• calculateOrderSubtotal(order);
• calculateSubtotalTaxes(subtotal, taxRates);

Each of these functions are named for what they are meant to do. Each can be investigated independently to determine what the issue is. We can also write tests with different values to make sure they are behaving as expected. Note that for getZipcodeTaxRates(userZipcode) a comment was added to explain a specific case when it might return a value other than what is expected.

Here is an example test for the calculateOrderSubtotal function.

test("calculate subtotal", () => {
    const OrderMock = {
        id: 1234,
        total: 1500,
        shippingCost: 999
    };

    const result = calculateOrderSubtotal(OrderMock1);
    expect(result).toEqual(2499);
});

We can test the calculateOrderSubtotal function with several other values to make sure we are getting the results we want in all cases. We can do the same for the other functions to ensure we are getting exactly what we want.

By encapsulating the code into properly named functions, it becomes easier to read, maintain and test code. Comments should be reserved for exceptions where the code is doing something unexpected. Another case for comments is when we are breaking convention due to some limitation. For any other case, the code and function names should be sufficient.

---