Removing Commented-Out Code

9 min read

Clean Code and Readability — review Java source for disabled implementation kept in comments instead of relying on version control.

1. Introduction

Commented-out code is executable code that has been disabled by placing it inside comments instead of deleting it.

It commonly appears like this:

JAVA
public void createOrder(Order order) {
    validateOrder(order);
    orderRepository.save(order);
    // inventoryService.reserve(order);
    // notificationService.sendOrderConfirmation(order);
}

Developers usually leave code commented out because:

  • They may need it again later.
  • They are testing an alternative implementation.
  • A feature has been temporarily disabled.
  • A migration replaced the original implementation.
  • They are afraid to delete code written by another developer.
  • They want to preserve old logic for reference.
  • They expect to restore the code after debugging.

During local development, temporarily commenting code can sometimes be useful.

The problem begins when commented-out implementation is committed to the main codebase and remains there indefinitely.

In a production Java project, commented-out code creates uncertainty:

  • Is this code still required?
  • Why was it disabled?
  • Should it be restored?
  • Is the active implementation incomplete?
  • Is the comment documenting history or abandoned work?
  • Does some business requirement still depend on it?

Modern projects already use Git, GitHub, GitLab, Bitbucket, Azure DevOps, or another version-control system.

Deleted code can be recovered from history when genuinely needed.

Therefore, production source code should normally contain the implementation that is currently supported—not several previous implementations preserved as comments.

2. What This Topic Means

Removing commented-out code means deleting disabled executable code from production source once it is no longer part of the intended implementation.

Consider:

JAVA
public PaymentResponse processPayment(PaymentRequest request) {
    validate(request);
    PaymentResponse response = paymentGateway.process(request);
    // PaymentResponse response = legacyPaymentGateway.process(request);
    // auditLegacyPayment(response);
    // sendLegacyNotification(response);
    return response;
}

The commented lines are not documentation.

They are an obsolete implementation.

A developer reading the method must now understand both:

  • The active payment flow
  • The inactive payment flow

even though only one affects runtime behavior.

Removing commented-out code does not mean removing useful comments.

Good comments can still explain:

  • Non-obvious business decisions
  • Framework limitations
  • Workarounds
  • Security assumptions
  • External-system constraints
  • Why an unusual implementation is necessary

For example:

JAVA
// The payment provider requires amounts in minor currency units.
long amountInPaise = amount.multiply(BigDecimal.valueOf(100)).longValueExact();

That comment explains why the code behaves in a particular way.

This is different from:

JAVA
// long amount = request.getAmount();
// paymentRepository.save(payment);
// sendNotification(payment);

which merely preserves disabled code.

During code review, the reviewer should distinguish between useful explanatory comments and obsolete executable code hidden inside comments.

3. Why It Matters in Real Projects

Readability

Commented-out code interrupts normal reading flow.

Consider:

JAVA
public void updateCustomer(Customer customer) {
    validateCustomer(customer);
    // if (customer.isPremium()) {
    //     premiumCustomerService.update(customer);
    // }
    customerRepository.save(customer);
}

A reviewer naturally stops and asks:

Why is premium customer handling disabled?

Even if that behavior was removed years ago, the comments make it appear potentially relevant.

Removing it makes the current implementation immediately clearer.

Maintainability

Commented-out code becomes stale.

If active code evolves while commented code remains unchanged, restoring it later may introduce:

  • Old method signatures
  • Removed dependencies
  • Outdated business rules
  • Deprecated APIs
  • Invalid assumptions
  • Security problems

Keeping code commented out does not preserve a reliable backup.

It preserves an increasingly inaccurate snapshot.

Debugging

During production incidents, engineers search code quickly.

Commented-out code can appear in search results and mislead investigation.

A developer searching for:

JAVA
sendRefundNotification

may find several commented references and spend time determining which path is actually active.

Reliability

A future developer may uncomment old code without understanding why it was disabled.

This can restore outdated behavior.

Team Development

Large commented blocks create uncertainty in Pull Requests.

Reviewers may ask:

  • Should this be reviewed too?
  • Is the code temporary?
  • Is another ticket supposed to restore it?
  • Why wasn't it deleted?

Removing obsolete code makes ownership and intent clearer.

Performance

Commented-out code itself does not execute, so it has no direct runtime performance cost.

Performance should not be invented as a concern here.

The main cost is developer cognitive load and maintenance effort.

4. Core Concept

The core principle is:

Source code should describe the system that currently exists. Version control should preserve the systems that existed previously.

Suppose a payment service originally contained:

JAVA
paymentGatewayV1.process(request);

The application is migrated to:

JAVA
paymentGatewayV2.process(request);

Keeping both:

JAVA
paymentGatewayV2.process(request);
// paymentGatewayV1.process(request);

does not provide meaningful safety.

Git already contains:

  • The previous implementation
  • The commit where it changed
  • The developer who changed it
  • The Pull Request discussion
  • The reason for the migration
  • The original tests

That historical information is far more useful than an isolated commented line.

Comments Should Explain Intent, Not Preserve Implementations

Useful:

JAVA
// Retry only network failures. Business validation failures must not be retried.

Not useful:

JAVA
// retryPayment(payment);
// Thread.sleep(1000);
// retryPayment(payment);

Temporary Comments Should Stay Temporary

During development, a developer might temporarily write:

JAVA
// fraudClient.check(payment);

to debug an issue.

That can be acceptable locally.

Before committing the Pull Request, the code should either:

  • Be restored
  • Be deleted
  • Be replaced with a controlled feature flag if runtime switching is genuinely required

Comments are not a feature-management mechanism.

5. Important Rules

  • Delete obsolete executable code instead of commenting it out permanently.
  • Use Git history when previous implementations need to be inspected.
  • Do not commit large commented code blocks.
  • Do not use comments as a substitute for feature flags.
  • Do not preserve old implementations inside methods "just in case."
  • Keep comments that explain why, not comments that merely contain disabled Java statements.
  • Remove temporary debugging code before opening a Pull Request.
  • Remove commented logging statements that are no longer useful.
  • Remove commented imports and dependency declarations.
  • Remove commented configuration after confirming it is obsolete.
  • Use a ticket or issue tracker for future work instead of commented code placeholders.
  • Add a meaningful TODO only when there is a real follow-up action and ownership.
  • Avoid TODO comments that silently become permanent.
  • If code must be temporarily disabled in production, use an explicit mechanism such as configuration or a feature flag.
  • Document the reason and removal date for temporary switches.
  • During review, ask why the code is commented instead of deleted.
  • Do not automatically delete explanatory comments that capture important business context.
  • Keep comments synchronized with active behavior.

6. Bad Code Example

Consider a Spring Boot order service that recently migrated to a new inventory reservation flow.

JAVA
@Service
public class OrderService {
    private final OrderRepository orderRepository;
    private final InventoryService inventoryService;
    private final NotificationService notificationService;
    public OrderService(OrderRepository orderRepository, InventoryService inventoryService, NotificationService notificationService) {
        this.orderRepository = orderRepository;
        this.inventoryService = inventoryService;
        this.notificationService = notificationService;
    }
    public Order placeOrder(Order order) {
        validateOrder(order);
        // Old inventory flow
        // List<OrderItem> items = order.getItems();
        // for (OrderItem item : items) {
        //     Inventory inventory = inventoryRepository.findByProductId(item.getProductId());
        //     if (inventory.getQuantity() < item.getQuantity()) {
        //         throw new IllegalStateException("Insufficient inventory");
        //     }
        //     inventory.setQuantity(inventory.getQuantity() - item.getQuantity());
        //     inventoryRepository.save(inventory);
        // }
        inventoryService.reserveInventory(order);
        order.setStatus(OrderStatus.CONFIRMED);
        Order savedOrder = orderRepository.save(order);
        // emailService.sendOrderConfirmation(savedOrder);
        notificationService.sendOrderConfirmation(savedOrder);
        // System.out.println("Order placed: " + savedOrder.getId());
        return savedOrder;
    }
    private void validateOrder(Order order) {
        if (order == null) {
            throw new IllegalArgumentException("Order is required");
        }
        if (order.getItems() == null || order.getItems().isEmpty()) {
            throw new IllegalArgumentException("Order must contain at least one item");
        }
    }
}

The active code is relatively small, but a large part of the method consists of obsolete implementation preserved in comments.

7. Problems in the Bad Code

Large Commented-Out Implementation

The old inventory workflow is no longer executed.

Keeping it inside the method forces readers to understand code that cannot affect current behavior.

Duplicate Historical Context

The old implementation is already available in version control.

Keeping another copy in comments provides little value.

Misleading Business Logic

The commented code suggests that inventory reservation may still need to happen directly inside OrderService.

A new developer may incorrectly assume that:

JAVA
inventoryService.reserveInventory(order);

does not completely replace the old flow.

Stale Dependencies

The commented code references:

JAVA
inventoryRepository

which is not even present in the current class.

This demonstrates how commented code quickly becomes outdated.

Obsolete Notification Path

This line remains:

JAVA
// emailService.sendOrderConfirmation(savedOrder);

while the current application uses:

JAVA
notificationService.sendOrderConfirmation(savedOrder);

The comment creates uncertainty about whether email notification is still required separately.

Debugging Artifact

This statement:

JAVA
// System.out.println("Order placed: " + savedOrder.getId());

is a temporary debugging artifact that should not remain in production source.

Review Noise

A Pull Request touching this method requires reviewers to visually separate active Java from dead comments.

Future Reactivation Risk

A developer may later uncomment the old inventory code without realizing:

  • Inventory locking has changed
  • Repository APIs have changed
  • Transactions have changed
  • Concurrency requirements have changed

8. Code Review Findings

A senior reviewer should notice:

  • A large obsolete inventory implementation remains commented inside the business method.
  • The current inventory behavior appears to have moved to InventoryService.
  • The old repository-based implementation should be deleted if migration is complete.
  • The commented email call should be removed unless there is a specific unresolved requirement.
  • The commented System.out.println should be deleted.
  • Git history is a better location for the previous implementation.
  • If the old inventory implementation is needed for rollback, that rollback strategy should be explicit rather than hidden in comments.
  • The reviewer should confirm that inventoryService.reserveInventory(order) fully replaces the old workflow.
  • Comments should explain any non-obvious inventory design decisions rather than preserve executable code.
  • The PR should leave the method showing only the active processing path.

9. Reviewer Comment Example

The old inventory implementation is fully commented out and makes this method harder to follow. If the migration is complete, please remove it and rely on Git history for the previous version.

Is the commented emailService call still part of an upcoming requirement? If not, please delete it so the supported notification flow is unambiguous.

Please remove the commented System.out.println. Temporary debugging statements should not remain in production source.

If we still need rollback capability for the old inventory flow, can we implement that explicitly through configuration or a feature flag instead of preserving disabled code in comments?

A short comment explaining why inventory reservation is delegated to InventoryService would be more useful than keeping the old implementation here.

10. Improved Code

JAVA
@Service
public class OrderService {
    private final OrderRepository orderRepository;
    private final InventoryService inventoryService;
    private final NotificationService notificationService;
    public OrderService(OrderRepository orderRepository, InventoryService inventoryService, NotificationService notificationService) {
        this.orderRepository = orderRepository;
        this.inventoryService = inventoryService;
        this.notificationService = notificationService;
    }
    public Order placeOrder(Order order) {
        validateOrder(order);
        inventoryService.reserveInventory(order);
        order.setStatus(OrderStatus.CONFIRMED);
        Order savedOrder = orderRepository.save(order);
        notificationService.sendOrderConfirmation(savedOrder);
        return savedOrder;
    }
    private void validateOrder(Order order) {
        if (order == null) {
            throw new IllegalArgumentException("Order is required");
        }
        if (order.getItems() == null || order.getItems().isEmpty()) {
            throw new IllegalArgumentException("Order must contain at least one item");
        }
    }
}

If the reason for delegating inventory management is important but non-obvious, a focused comment may be useful:

JAVA
// InventoryService owns reservation and concurrency control for all order channels.
inventoryService.reserveInventory(order);

That comment explains architectural intent.

It does not preserve an obsolete implementation.

11. Improved Code Explanation

Only Active Behavior Remains

The placeOrder method now shows the current business flow directly:

  1. Validate order
  2. Reserve inventory
  3. Confirm order
  4. Persist order
  5. Send notification

Historical Code Removed

The old repository-based inventory implementation is no longer mixed with the current design.

Notification Flow Is Clear

There is one supported call:

JAVA
notificationService.sendOrderConfirmation(savedOrder);

No commented alternative remains.

Debugging Noise Removed

The temporary System.out.println is gone.

Architecture Is Easier to Understand

A developer can immediately see that inventory responsibility belongs to:

JAVA
InventoryService

instead of trying to infer whether the old implementation is still relevant.

Git Becomes the Historical Record

If the previous implementation is needed during investigation, the commit history can show the complete version with proper context.

12. Bad Code vs Improved Code

AspectBad CodeImproved Code
ReadabilityActive and inactive implementations mixed togetherOnly current behavior visible
MaintainabilityOld comments become staleCurrent implementation remains focused
DebuggingSearch results include obsolete statementsSearch results reflect active code
ReliabilityOld logic can be accidentally restoredUnsupported path is removed
ReviewabilityReviewer must skip large comment blocksReviewer sees actual behavior immediately
DocumentationComments preserve outdated implementationComments can explain current design intent
Version historyDuplicated inside source fileGit remains the authoritative history

13. Real Project Scenario

Consider a healthcare microservice that calculates patient claim eligibility.

The original implementation contains:

JAVA
if (claim.getAmount().compareTo(MAX_AMOUNT) <= 0) {
    approveClaim(claim);
}

Later, the organization introduces a centralized rules engine:

JAVA
eligibilityClient.evaluate(claim);

During migration, the developer comments out the old rule instead of deleting it:

JAVA
// if (claim.getAmount().compareTo(MAX_AMOUNT) <= 0) {
//     approveClaim(claim);
// }
EligibilityDecision decision = eligibilityClient.evaluate(claim);

Months later, the maximum-amount rule changes in the centralized rules engine.

The commented Java implementation still contains the old threshold.

During an incident, another developer sees the commented code and assumes it represents the current business requirement.

They spend time investigating why the rules engine behaves differently.

Worse, someone may uncomment the old code during emergency troubleshooting.

The application would then use outdated eligibility rules.

The correct approach after migration is complete is to remove the old Java implementation.

If the architectural reason matters, document it:

JAVA
// Claim eligibility is owned by the centralized rules service.
EligibilityDecision decision = eligibilityClient.evaluate(claim);

The comment communicates current ownership instead of preserving obsolete behavior.

14. Production Impact

Commented-out code does not execute directly, so it normally does not cause runtime failures or performance degradation by itself.

Its production impact is mostly indirect.

Incorrect Emergency Fixes

During production incidents, developers sometimes make fast changes.

Commented old code may be restored because it appears to provide a quick fallback.

If that code is outdated, the hotfix can introduce new defects.

Misunderstood Business Rules

Commented code may contain old thresholds, validations, or workflows that no longer reflect current requirements.

Slower Incident Investigation

Search results and code navigation include inactive references that developers must eliminate from consideration.

Maintenance Errors

A developer may update both active and commented code unnecessarily, increasing PR size and review effort.

Security Risk Through Reactivation

Commented authentication or authorization code may contain obsolete security rules.

Uncommenting it later can restore a vulnerable path.

Operational Confusion

Commented configuration or old integration calls can make engineers believe that unsupported systems remain part of the architecture.

15. Common Developer Mistakes

Keeping Code "For Reference"

Example:

JAVA
// Previous implementation
// ...

Git already provides better historical reference.

Commenting Out Code Instead of Removing It

Developers often avoid deletion because deletion feels permanent.

In version-controlled projects, it is not permanent.

Leaving Temporary Debugging Changes

Example:

JAVA
// log.debug("Customer: {}", customer);
// System.out.println(customer);

Temporary investigation artifacts remain indefinitely.

Commenting Old Logic During Refactoring

A developer extracts logic into a new service but leaves the original code commented inside the caller.

Commenting Feature Behavior Instead of Using Feature Flags

Bad:

JAVA
// fraudService.check(order);

If runtime enable/disable behavior is required, a controlled mechanism should be used.

Commented Imports

Example:

JAVA
// import com.example.legacy.LegacyPaymentClient;

These add no value.

Commented Tests

Tests are sometimes disabled by commenting entire test methods instead of deleting them or using an explicit temporary mechanism with clear justification.

Long-Lived TODO Comments

Example:

JAVA
// TODO uncomment this after migration

without a ticket, owner, or deadline.

Preserving Alternative Algorithms

Developers sometimes keep two complete implementations because they are unsure which one is better.

Only the selected implementation should remain in production source unless both have an active purpose.

16. Edge Cases

Temporarily Disabled Code During Active Development

Commenting code locally while debugging is normal.

The issue is committing it as permanent source.

Short-Lived Migration Work

During an active migration, old behavior may need to remain available temporarily.

Use explicit configuration or feature flags if runtime rollback is required.

Generated Code

Generated source files may contain unusual comments inserted by code generators.

Do not manually clean generated files unless the generation process itself is changed.

Educational or Example Code

Tutorials may intentionally show commented alternatives.

Production application code has different expectations.

Code Snippets in Javadoc

Javadoc may contain examples that look like executable code.

These are documentation, not commented-out production implementation.

Regulatory or Audit Explanation

Some projects require documenting why a particular rule changed.

The correct solution is usually:

  • Documentation
  • ADR
  • Ticket
  • Commit history
  • Audit record

not leaving the old implementation commented out.

Temporarily Disabled Tests

A test may occasionally need temporary disabling.

Prefer explicit mechanisms such as:

JAVA
@Disabled("Blocked by issue PAY-1421")

rather than commenting out the entire test body.

This makes the state visible to tooling.

17. Performance Considerations

Commented-out code does not execute.

Therefore it normally has:

  • No runtime CPU cost
  • No runtime memory cost
  • No database cost
  • No external API cost

Performance is not the primary concern for this topic.

However, commented code can have indirect engineering costs.

Larger Source Files

Large comment blocks make files harder to navigate.

Slower Human Review

Developers spend more time identifying relevant code.

Static Search Noise

Search results may include inactive methods, SQL, API calls, and configuration names.

Build Performance

Normal Java comments have negligible impact on production execution because the compiler does not turn them into executable bytecode.

The reviewer should therefore focus on maintainability rather than inventing runtime-performance claims.

18. Security Considerations

Commented-out code is not directly executed, so it is not automatically a security vulnerability.

However, there are several security-related concerns.

Commented Secrets

Never leave credentials in comments.

Bad:

JAVA
// String apiKey = "prod-secret-key-123";

Secrets remain visible in source history and repository access.

Old Security Logic

Commented code such as:

JAVA
// if (user.isAdmin()) {
//     allowAccess();
// }

can create confusion about current authorization design.

Obsolete Authentication Implementations

Keeping old token validation or password logic as comments may encourage accidental reuse.

Sensitive Debugging Statements

Bad:

JAVA
// log.info("Token: {}", accessToken);

Even though currently commented, restoring such code later could expose sensitive information.

Git History Consideration

Deleting a committed secret from current source does not erase it from Git history.

Credential rotation may still be required.

Reviewer Focus

When commented code contains:

  • Tokens
  • Passwords
  • API keys
  • Personal data
  • Security rules

the reviewer should treat cleanup seriously rather than considering it mere formatting.

19. Testing Considerations

Removing commented-out code should not change runtime behavior because comments are not executed.

Therefore extensive new behavioral tests are usually unnecessary solely because comments were deleted.

However, the surrounding active implementation should already have appropriate tests.

Verify Current Behavior

If the commented code represents an old implementation replaced by a new one, verify that the new path is tested.

For example:

JAVA
@Test
void shouldReserveInventoryUsingInventoryService() {
    Order order = validOrder();
    when(orderRepository.save(order)).thenReturn(order);
    orderService.placeOrder(order);
    verify(inventoryService).reserveInventory(order);
}

Migration Verification

If old code was retained as a fallback, ensure the new implementation has:

  • Unit tests
  • Integration tests
  • Relevant end-to-end coverage

before removing historical comments.

No Tests for Commented Code

Do not maintain tests for behavior that the application no longer supports.

Disabled Tests

If a test itself has been commented out, determine whether:

  • The behavior is obsolete
  • The test is broken
  • The implementation is broken
  • A real issue should be created

Do not let commented test suites remain indefinitely.

20. Refactoring Guidelines

Step 1: Identify Commented Executable Code

Search for large comment blocks containing:

  • Method calls
  • Conditions
  • Loops
  • SQL
  • Object creation
  • Old service calls

Step 2: Understand Why It Was Commented

Check:

  • Git history
  • Pull Request discussion
  • Issue tracker
  • Migration documentation

Step 3: Confirm the Code Is Not Required

Determine whether it represents:

  • Obsolete behavior
  • Temporary debugging
  • Incomplete feature work
  • Required rollback capability

Step 4: Ensure Current Behavior Is Protected

Verify tests around the active implementation.

Step 5: Delete the Commented Code

Do not replace it with another comment such as:

JAVA
// Old code removed.

That provides little value.

Step 6: Keep Useful Explanatory Context

If an unusual design decision needs explanation, replace the old implementation with a short intent-focused comment.

Also remove:

  • Commented imports
  • Commented variables
  • Commented configuration
  • Obsolete TODOs

Step 8: Review the Diff

A cleanup PR should make the active code easier to see.

21. Best Practices

  • Treat Git as the historical backup.
  • Delete disabled implementations once they are obsolete.
  • Keep comments focused on reasoning and constraints.
  • Use ADRs for architectural history.
  • Use tickets for future work.
  • Use feature flags for controlled runtime behavior.
  • Attach removal dates to temporary feature flags.
  • Remove debugging comments before merge.
  • Prefer @Disabled with a tracked reason over commenting out tests.
  • Keep comments near the code they explain.
  • Update comments when behavior changes.
  • Remove stale comments during normal refactoring.
  • Review commented code as part of every Pull Request.
  • Keep production methods visually focused on executable behavior.
  • Question comments that contain more Java code than explanation.

22. Practices to Avoid

Large Comment Blocks

Avoid:

JAVA
// if (...) {
//     ...
// }

when the implementation is obsolete.

Commenting Instead of Version Control

Do not preserve multiple generations of code in the same source file.

Commented Debug Logs

Avoid:

JAVA
// System.out.println(...);
// log.info(...);

when they are no longer required.

Commented Imports

They serve no useful purpose.

Commented Secrets

Never preserve credentials or tokens in source comments.

"Maybe Later" Implementations

Avoid retaining code merely because it could theoretically be useful one day.

Fake Feature Flags

Avoid:

JAVA
// paymentService.processV2(request);
paymentService.processV1(request);

for switching implementations.

Commented-Out Tests

Do not hide failing tests by commenting them.

Comments That Repeat the Code

Avoid:

JAVA
// Save the customer
customerRepository.save(customer);

unless the comment provides additional context.

23. Code Review Checklist

  • Does this file contain commented-out executable code?
  • Is the commented code still required?
  • Why was it commented instead of deleted?
  • Is the previous implementation already available in Git history?
  • Does this comment explain intent or merely preserve old Java statements?
  • Is the commented code part of an unfinished migration?
  • Should runtime switching use a feature flag instead?
  • Are there commented debugging statements?
  • Are there commented imports?
  • Are there commented configuration properties?
  • Does the commented block contain outdated business rules?
  • Could a future developer mistakenly restore obsolete behavior?
  • Does the commented code contain security-sensitive information?
  • Are any secrets or credentials present in comments?
  • Are tests commented out instead of explicitly disabled or removed?
  • Is there a TODO with no ticket or owner?
  • Can a large comment block be replaced with a short explanation of current design intent?
  • Does removing the comments leave current behavior unchanged?
  • Are active paths sufficiently tested?
  • Will the resulting method become easier to read after cleanup?

24. Common Pull Request Review Comments

  1. *This block is an old implementation and is already preserved in Git history. Please remove it rather than keeping it commented out.*
  1. *Is there a reason we still need this commented service call? If the old integration is retired, I would delete it to keep the supported path clear.*
  1. *Please remove the commented System.out.println; this looks like a local debugging artifact.*
  1. *If we need to switch between these implementations at runtime, let's use an explicit feature flag rather than commenting one version out.*
  1. *This TODO says to uncomment the code later but does not reference any follow-up work. Can we either link a ticket or remove the obsolete block?*
  1. *The commented code contains the previous authorization logic. Please remove it so there is no ambiguity about which security rule is current.*
  1. *This old SQL query is no longer used. Git can provide the historical query if we need it later.*
  1. *Instead of preserving the old implementation, could we add a short comment explaining why the new service owns this responsibility?*
  1. *Please don't comment out the failing test. Either fix it, explicitly disable it with a tracked reason, or remove it if the behavior no longer exists.*
  1. *This commented configuration includes a credential placeholder. Please remove it completely rather than keeping sensitive configuration examples in application source.*

25. Code Review Exercise

Review the following Spring Boot payment service.

Identify:

  • Problems
  • Code smells
  • Risks
  • Improvements

Do not read the solution until completing your review.

JAVA
@Service
public class PaymentService {
    private final PaymentRepository paymentRepository;
    private final PaymentGateway paymentGateway;
    public PaymentService(PaymentRepository paymentRepository, PaymentGateway paymentGateway) {
        this.paymentRepository = paymentRepository;
        this.paymentGateway = paymentGateway;
    }
    public Payment processPayment(PaymentRequest request) {
        validateRequest(request);
        // Old gateway implementation
        // LegacyPaymentRequest legacyRequest = new LegacyPaymentRequest();
        // legacyRequest.setAmount(request.getAmount());
        // legacyRequest.setCustomerId(request.getCustomerId());
        // LegacyPaymentResponse legacyResponse = legacyGateway.charge(legacyRequest);
        // if (!legacyResponse.isSuccessful()) {
        //     throw new PaymentException("Legacy payment failed");
        // }
        PaymentGatewayResponse gatewayResponse = paymentGateway.charge(request);
        if (!gatewayResponse.isSuccessful()) {
            throw new PaymentException("Payment failed");
        }
        Payment payment = new Payment();
        payment.setCustomerId(request.getCustomerId());
        payment.setAmount(request.getAmount());
        payment.setStatus(PaymentStatus.COMPLETED);
        // payment.setGateway("LEGACY");
        payment.setGateway("PRIMARY");
        Payment savedPayment = paymentRepository.save(payment);
        // System.out.println("Payment ID: " + savedPayment.getId());
        return savedPayment;
    }
    private void validateRequest(PaymentRequest request) {
        if (request == null) {
            throw new IllegalArgumentException("Payment request is required");
        }
        // if (request.getAmount() == null) {
        //     throw new IllegalArgumentException("Amount is required");
        // }
        if (request.getAmount() == null || request.getAmount().signum() <= 0) {
            throw new IllegalArgumentException("Amount must be positive");
        }
    }
}

Review the code for:

  • Commented legacy implementation
  • Stale business values
  • Duplicate historical validation
  • Debugging artifacts
  • Misleading comments
  • Potential future reactivation risk
  • Better use of version control

26. Exercise Solution

Problems Identified

1. Large Legacy Gateway Block

The method contains an entire previous payment flow:

JAVA
// LegacyPaymentRequest ...
// legacyGateway.charge(...)
// ...

If the legacy gateway has been retired, this implementation should be deleted.

2. Stale Gateway Assignment

This line remains:

JAVA
// payment.setGateway("LEGACY");

It creates unnecessary uncertainty about whether legacy payments are still supported.

3. Commented Debug Statement

This line:

JAVA
// System.out.println("Payment ID: " + savedPayment.getId());

is a temporary debugging artifact.

4. Old Validation Preserved

This block:

JAVA
// if (request.getAmount() == null) {
//     throw new IllegalArgumentException("Amount is required");
// }

is superseded by the active validation:

JAVA
if (request.getAmount() == null || request.getAmount().signum() <= 0)

The commented version has no current purpose.

5. Historical Code Mixed With Current Business Logic

A reader must compare the active and inactive payment paths even though only one is supported.

6. Risk of Restoring Outdated Behavior

The legacy code may no longer match:

  • Current gateway contracts
  • Current exception handling
  • Current security rules
  • Current persistence model

Uncommenting it later could introduce defects.

Improved Code

JAVA
@Service
public class PaymentService {
    private final PaymentRepository paymentRepository;
    private final PaymentGateway paymentGateway;
    public PaymentService(PaymentRepository paymentRepository, PaymentGateway paymentGateway) {
        this.paymentRepository = paymentRepository;
        this.paymentGateway = paymentGateway;
    }
    public Payment processPayment(PaymentRequest request) {
        validateRequest(request);
        PaymentGatewayResponse gatewayResponse = paymentGateway.charge(request);
        if (!gatewayResponse.isSuccessful()) {
            throw new PaymentException("Payment failed");
        }
        Payment payment = new Payment();
        payment.setCustomerId(request.getCustomerId());
        payment.setAmount(request.getAmount());
        payment.setStatus(PaymentStatus.COMPLETED);
        payment.setGateway("PRIMARY");
        return paymentRepository.save(payment);
    }
    private void validateRequest(PaymentRequest request) {
        if (request == null) {
            throw new IllegalArgumentException("Payment request is required");
        }
        if (request.getAmount() == null || request.getAmount().signum() <= 0) {
            throw new IllegalArgumentException("Amount must be positive");
        }
    }
}

If gateway identity is a domain value, an enum would be stronger:

JAVA
payment.setGateway(PaymentGatewayType.PRIMARY);

Why Each Change Is Useful

  • The active payment path is immediately visible.
  • Legacy implementation no longer distracts reviewers.
  • Obsolete validation is removed.
  • Debugging artifacts are removed.
  • Future developers are less likely to reactivate retired gateway logic.
  • Git remains available if historical implementation needs inspection.
  • Current business values can evolve without maintaining obsolete alternatives.

27. Interview Perspective

Removing commented-out code may appear simple, but interviewers use it to evaluate practical engineering discipline.

Java Interview

A candidate may be asked:

Why shouldn't we keep old code commented out?

A strong answer should explain:

  • Version control already preserves history.
  • Commented code becomes stale.
  • It increases cognitive load.
  • It creates ambiguity about supported behavior.

Spring Boot Interview

A service may contain a commented old repository call or integration.

The interviewer may ask whether it should stay for rollback.

A strong candidate should explain that rollback should be implemented through:

  • Deployment strategy
  • Version rollback
  • Configuration
  • Feature flags

not source-code comments.

Senior Developer Interview

A senior candidate may be asked:

A developer says, "Don't delete this; we may need it later." How would you respond?

A strong response is:

If the behavior is no longer required, delete it. Git preserves the previous implementation. If rollback is a real operational requirement, we should design an explicit rollback mechanism instead of storing inactive code in comments.

Code Review Interview

The candidate may be asked to write a professional review comment.

The expected feedback should be:

  • Respectful
  • Specific
  • Actionable
  • Based on maintainability rather than personal preference

28. Interview Questions and Answers

Basic Question

Question: Why is commented-out code considered a code smell?

Answer:

Because it mixes inactive implementation with current behavior. Developers must determine why it exists and whether it is still relevant. Since version control already preserves previous versions, old executable code normally should be deleted rather than stored inside comments.

Intermediate Question

Question: Is it ever acceptable to comment out code?

Answer:

Temporarily during local development or debugging, yes. It should normally not remain in merged production code. If behavior must be switched dynamically, a proper feature flag or configuration mechanism should be used instead.

Advanced Question

Question: Why is Git history better than keeping an old implementation commented in the source?

Answer:

Git preserves the complete historical context: the full file, surrounding changes, commit message, author, date, Pull Request, and related modifications. A commented block preserves only a partial and increasingly stale snapshot. Version control therefore provides more reliable history with less source-code noise.

Scenario-Based Question

Question: A team wants to keep the previous payment-provider code commented for emergency rollback. What would you recommend?

Answer:

If rollback is genuinely required, I would use a deployment rollback, version rollback, controlled feature flag, or configurable provider strategy. Commented-out code cannot be activated safely without modifying, rebuilding, testing, and redeploying the application, so it is not an effective rollback mechanism.

Code-Review Question

Question: You see 50 lines of commented code in a Pull Request. What would your review comment be?

Answer:

I would ask whether the implementation still serves any active purpose. If not, I would request deletion and mention that Git history already preserves it. If it represents planned work or rollback behavior, I would ask for an explicit mechanism or tracked issue rather than leaving disabled Java statements in the method.

Real-Project Question

Question: How do commented-out blocks affect production support even though they do not execute?

Answer:

They create search noise and ambiguity during incident investigation. Engineers may waste time analyzing obsolete paths or assume old business rules are still valid. In urgent situations, someone may also restore stale code incorrectly. Keeping the source aligned with supported behavior makes production debugging safer.

29. Quick Rule to Remember

If old code is needed for history, Git should store it—not comments inside the current implementation.

30. Final Takeaway

What the Developer Should Remember

Commented-out code is usually not documentation.

It is abandoned implementation mixed into current source.

Production Java code should clearly show what the system does today.

Developers should:

  • Delete obsolete implementation.
  • Use Git for history.
  • Remove local debugging artifacts before merge.
  • Avoid commented imports and configuration.
  • Do not preserve old business rules inside comments.
  • Use feature flags when runtime switching is genuinely required.
  • Keep explanatory comments focused on why the current code behaves as it does.
  • Remove stale TODO comments.
  • Avoid commenting out failing tests.
  • Keep current source aligned with current architecture.

What the Reviewer Should Check

During Pull Request review, ask:

  • Why is this code commented out?
  • Is it still needed?
  • Does Git already preserve it?
  • Is this unfinished work?
  • Is a feature flag more appropriate?
  • Could the commented logic become stale?
  • Could someone accidentally restore outdated behavior?
  • Does the comment contain a secret or sensitive value?
  • Are debugging statements being preserved?
  • Are tests commented out?
  • Can the block be deleted entirely?
  • Would a short architectural comment be more useful?

What Should Be Avoided in Production Code

Avoid leaving:

JAVA
// oldService.process();
// newServiceV1.process();
// previousQuery();
// System.out.println(...);

Avoid storing entire previous implementations beside current code.

Avoid using comments as deployment switches.

Avoid leaving old authorization, validation, payment, or database logic commented for future reuse.

Avoid treating comments as a substitute for Git history.

The production-quality approach is straightforward:

Keep executable source focused on supported behavior, keep comments focused on intent, and let version control preserve everything that came before.