Long Method — When a Function Tries to Do Everything

You open a function. You scroll. You keep scrolling. By the time you reach the end, you've forgotten what the top looked like. Sound familiar? That's a Long Method — one of the most common code smells in any codebase. But here's the thing: it's not really about line count. A 50-line function can be perfectly clean. A 15-line function can be a god function in disguise. The real issue is how many responsibilities it holds.

How to spot it

The signals are usually subtle at first:

The function name contains and, also, or with — fetchAndSaveUser, validateAndSendEmail
There are comment blocks inside acting as section headers — // 1. Validate, // 2. Save
You can't write a single unit test without mocking half the app
Changing one scenario forces you to edit the middle of a big function

The comment-as-section-header is the most honest tell. If you felt the need to label sections, each section deserves its own function.

A real example

Here's a typical handleJobApplication method you might find in a NestJS service:

// ❌ God Function — 5 responsibilities in one place
async function handleJobApplication(dto: ApplyJobDto, userId: string) {
  // 1. Validate input
  if (!dto.jobId || !dto.coverLetter) {
    throw new BadRequestException('Missing fields');
  }

  // 2. Check for duplicate application
  const existing = await this.applicationRepo.findOne({
    where: { jobId: dto.jobId, userId },
  });
  if (existing) throw new ConflictException('Already applied');

  // 3. Load resume
  const resume = await this.fileService.getFile(dto.resumeFileId);
  if (!resume) throw new NotFoundException('Resume not found');

  // 4. Save application
  const application = this.applicationRepo.create({
    jobId: dto.jobId,
    userId,
    coverLetter: dto.coverLetter,
    resumeUrl: resume.url,
    status: ApplicationStatus.PENDING,
  });
  await this.applicationRepo.save(application);

  // 5. Send confirmation email
  const user = await this.userRepo.findOne({ where: { id: userId } });
  await this.mailer.sendMail({
    to: user.email,
    subject: 'Application received',
    template: 'job-application',
    context: { name: user.name, jobId: dto.jobId },
  });

  return application;
}

Each of those five comment sections is a hidden function waiting to be extracted.

The refactored version

After splitting by responsibility, the top-level function becomes almost self-documenting:

// ✅ Each function does one thing
async function handleJobApplication(dto: ApplyJobDto, userId: string) {
  await this.validateApplication(dto, userId);
  const resume = await this.getResumeOrThrow(dto.resumeFileId);
  const application = await this.saveApplication(dto, userId, resume);
  await this.notifyApplicant(userId, dto.jobId);
  return application;
}

private async validateApplication(dto: ApplyJobDto, userId: string) {
  if (!dto.jobId || !dto.coverLetter) {
    throw new BadRequestException('Missing fields');
  }
  const existing = await this.applicationRepo.findOne({
    where: { jobId: dto.jobId, userId },
  });
  if (existing) throw new ConflictException('Already applied');
}

private async getResumeOrThrow(fileId: string) {
  const resume = await this.fileService.getFile(fileId);
  if (!resume) throw new NotFoundException('Resume not found');
  return resume;
}

private async saveApplication(
  dto: ApplyJobDto,
  userId: string,
  resume: FileEntity,
) {
  const application = this.applicationRepo.create({
    jobId: dto.jobId,
    userId,
    coverLetter: dto.coverLetter,
    resumeUrl: resume.url,
    status: ApplicationStatus.PENDING,
  });
  return this.applicationRepo.save(application);
}

private async notifyApplicant(userId: string, jobId: string) {
  const user = await this.userRepo.findOne({ where: { id: userId } });
  await this.mailer.sendMail({
    to: user.email,
    subject: 'Application received',
    template: 'job-application',
    context: { name: user.name, jobId },
  });
}

What you actually gain

The refactored version isn't just cleaner to look at — it changes how you work with the code:

Testing — each piece can be tested in complete isolation
Reuse — notifyApplicant can now be called from other flows
Change safety — updating email logic only touches notifyApplicant, nothing else
Readability — handleJobApplication reads like a summary of the process

The top-level function becomes a table of contents. You understand the flow at a glance, and only dive into the details you actually need.

The rule of thumb

A function should be describable in one sentence — without using the word "and". If you catch yourself writing // Step 3 inside a function body, stop. That comment is telling you something.