TypeScript functions within functions vs. private functions

Question

Here is a hypothetical example:

I have a market.service.ts with 2 functions called buy() and sell() which are each 100 lines of code

export class ShopService {
  public async buy() {
    // 100 lines of code
  }
  public async sell() {
    // 100 lines of code
  }
}

I want to refactor these two into 2 main functions which each will call some mini functions. Each of these two are being called in a controller, so i can not change the public api for this service and buy() and sell() should have the same api.

Here is my question:

What are the pros and cons of each of these approaches? (for context, my main concern is code readability and not performance at all cost! also, this class has 100% test coverage)

Here are the two approaches i can take for this:

1. I can define the mini functions as private functions inside this service class and just call them once.

export class ShopService {
  public async buy() {
    this.checkBuyMarket();
    this.checkIfUserCanBuy();
    this.makeTheBuyOrder();
  }
  public async sell() {
    this.checkSellMarket();
    this.checkIfUserCanSell();
    this.makeTheSellOrder();
  }

  private async checkSellMarket() {
    //30 lines of code
  }
  private async checkBuyMarket() {
    //30 lines of code
  }
  private async checkIfUserCanBuy() {
    //30 lines of code
  }
  private async checkIfUserCanSell() {
    //30 lines of code
  }
  private async makeTheBuyOrder() {
    //30 lines of code
  }
  private async makeTheSellOrder() {
    //30 lines of code
  }
}

2. I can define these functions that are called only once, within buy() or sell() and call them there.

export class ShopService {
  public async buy() {
    checkBuyMarket();
    checkIfUserCanBuy();
    makeTheBuyOrder();

    async function checkBuyMarket() {
      //30 lines of code
    }
    async function checkIfUserCanBuy() {
      //30 lines of code
    }
    async function makeTheBuyOrder() {
      //30 lines of code
    }
  }
  public async sell() {
    checkSellMarket();
    checkIfUserCanSell();
    makeTheSellOrder();

    async function checkSellMarket() {
      //30 lines of code
    }
    async function checkIfUserCanSell() {
      //30 lines of code
    }
    async function makeTheSellOrder() {
      //30 lines of code
    }
  }
}

Answer 1

Both approaches have their pros and cons, and the choice between them often comes down to personal preference and the specific context of your project. But as you said your priority is code readability then I would say you should prefer Private Methods. It has code readability, and code can be reused and can also encapsulate.

Ensure that you're not introducing code duplication. If similar logic is needed in both buy and sell, consider refactoring common functionality into shared private methods. Also, a good practice is to use private methods for more complex logic and using inline functions for simpler steps.

Answer 2

I strongly recommend version 1, and I do not think this is opinion-based. Version 2 wrongly communicates an intent and a complication that you do not have, and this is objectively a readability-issue.

The problem is that to understand an inner function, you must read the whole enclosing function, too, making your buy and sell functions effectively 100-line functions. Why is that?

Defining a function h inside a function f allows you to:

• access local variables of f
• return the function h with the captured variables local to f to be used outside of f
• use h as a callback-argument to APIs that f uses, making use of local variables in f

All are legitimate programming techniques, but they make f and h more complex to understand. Now, if h does not do any of that, a reader will think that some of these must be happening, so they have to scan the whole source-code of f and h to see that this is not happening. Worse: a maintainer might think it OK to modify h or f in such a way that they do do some of these three things, and it won't be easily visible in the diff, making your mental model of the functions wrong.